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.

2 files changed, 20 insertions, 12 deletions

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();