[poppler] poppler/SignatureHandler.cc poppler/SignatureHandler.h

[GitLab Mirror]

 poppler/SignatureHandler.cc |   30 +++++++++++++++++++-----------
 poppler/SignatureHandler.h  |    2 +-
 2 files changed, 20 insertions(+), 12 deletions(-)

New commits:
commit ede53de1654afe86e25fa22c7e9d50003a217d5a
Author: Sune Vuorela <sune at vuorela.dk>
Date:   Mon Mar 13 14:48:21 2023 +0100

    Valgrind cleanup of signing
    
    signing_cert is owned by us while verifying, but the data we were
    writing to it while signing was not (directly) owned by us, so don't
    free it. Or well. Don't write it to a class member.

diff --git a/poppler/SignatureHandler.cc b/poppler/SignatureHandler.cc
index 51c532c6..94240ef3 100644
--- a/poppler/SignatureHandler.cc
+++ b/poppler/SignatureHandler.cc
@@ -522,8 +522,6 @@ unsigned int SignatureHandler::digestLength(HashAlgorithm digestAlgId)
 
 std::string SignatureHandler::getSignerName()
 {
-    char *commonName;
-
     if (!NSS_IsInitialized()) {
         return {};
     }
@@ -531,16 +529,21 @@ std::string SignatureHandler::getSignerName()
     if (!signing_cert && !CMSSignerInfo) {
         return {};
     }
+    CERTCertificate *activeCert = nullptr;
 
     if (!signing_cert) {
-        signing_cert = NSS_CMSSignerInfo_GetSigningCertificate(CMSSignerInfo, CERT_GetDefaultCertDB());
+        // we are signing, and thus getting the name of the SignerInfo, not of the signing cert. Data owned by CMSSignerInfo
+        activeCert = NSS_CMSSignerInfo_GetSigningCertificate(CMSSignerInfo, CERT_GetDefaultCertDB());
+    } else {
+        // We are verifying. data owned by us.
+        activeCert = signing_cert;
     }
 
-    if (!signing_cert) {
+    if (!activeCert) {
         return {};
     }
 
-    commonName = CERT_GetCommonName(&signing_cert->subject);
+    char *commonName = CERT_GetCommonName(&activeCert->subject);
     if (!commonName) {
         return {};
     }
@@ -550,21 +553,26 @@ std::string SignatureHandler::getSignerName()
     return name;
 }
 
-const char *SignatureHandler::getSignerSubjectDN()
+std::string SignatureHandler::getSignerSubjectDN()
 {
     if (!signing_cert && !CMSSignerInfo) {
-        return nullptr;
+        return {};
     }
+    CERTCertificate *activeCert = nullptr;
 
     if (!signing_cert) {
-        signing_cert = NSS_CMSSignerInfo_GetSigningCertificate(CMSSignerInfo, CERT_GetDefaultCertDB());
+        // we are signing, and thus getting the name of the SignerInfo, not of the signing cert. Data owned by CMSSignerInfo
+        activeCert = NSS_CMSSignerInfo_GetSigningCertificate(CMSSignerInfo, CERT_GetDefaultCertDB());
+    } else {
+        // We are verifying. data owned by us.
+        activeCert = signing_cert;
     }
 
-    if (!signing_cert) {
-        return nullptr;
+    if (!activeCert) {
+        return {};
     }
 
-    return signing_cert->subjectName;
+    return activeCert->subjectName;
 }
 
 HashAlgorithm SignatureHandler::getHashAlgorithm()
diff --git a/poppler/SignatureHandler.h b/poppler/SignatureHandler.h
index fdc0ebd4..218000c4 100644
--- a/poppler/SignatureHandler.h
+++ b/poppler/SignatureHandler.h
@@ -51,7 +51,7 @@ public:
     ~SignatureHandler();
     time_t getSigningTime();
     std::string getSignerName();
-    const char *getSignerSubjectDN();
+    std::string getSignerSubjectDN();
     HashAlgorithm getHashAlgorithm();
     void updateHash(unsigned char *data_block, int data_len);
     void restartHash();