[poppler] 2 commits - poppler/SignatureHandler.cc

[GitLab Mirror]

 poppler/SignatureHandler.cc |   24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

New commits:
commit 829d03cccd77312886e0bc215366cfe600e65044
Author: Sune Vuorela <sune at vuorela.dk>
Date:   Mon Mar 13 14:05:59 2023 +0100

    Switch second digest buffer to not leak memory

diff --git a/poppler/SignatureHandler.cc b/poppler/SignatureHandler.cc
index d35862bb..059d8efb 100644
--- a/poppler/SignatureHandler.cc
+++ b/poppler/SignatureHandler.cc
@@ -1043,11 +1043,11 @@ std::unique_ptr<GooString> SignatureHandler::signDetached(const char *password)
     if (!hash_context) {
         return nullptr;
     }
-    unsigned char *digest_buffer = reinterpret_cast<unsigned char *>(PORT_Alloc(hash_length));
+    auto digest_buffer = std::vector<unsigned char>(hash_length);
     unsigned int result_len = 0;
-    HASH_End(hash_context.get(), digest_buffer, &result_len, hash_length);
+    HASH_End(hash_context.get(), digest_buffer.data(), &result_len, hash_length);
     SECItem digest;
-    digest.data = digest_buffer;
+    digest.data = digest_buffer.data();
     digest.len = result_len;
 
     /////////////////////////////////////
commit a691d8d5e6ce169325542b7912d61a215f412f28
Author: Sune Vuorela <sune at vuorela.dk>
Date:   Mon Mar 13 13:51:55 2023 +0100

    Don't manually handle the digest buffer memory
    
    Also fix length and content comparison to happen in the right order

diff --git a/poppler/SignatureHandler.cc b/poppler/SignatureHandler.cc
index d0f6e0fd..d35862bb 100644
--- a/poppler/SignatureHandler.cc
+++ b/poppler/SignatureHandler.cc
@@ -30,6 +30,7 @@
 #include "goo/gmem.h"
 
 #include <optional>
+#include <vector>
 
 #include <Error.h>
 
@@ -937,8 +938,6 @@ static SignatureValidationStatus NSS_SigTranslate(NSSCMSVerificationStatus nss_c
 
 SignatureValidationStatus SignatureHandler::validateSignature()
 {
-    unsigned char *digest_buffer = nullptr;
-
     if (!CMSSignedData) {
         return SIGNATURE_GENERIC_ERROR;
     }
@@ -951,14 +950,14 @@ SignatureValidationStatus SignatureHandler::validateSignature()
         return SIGNATURE_GENERIC_ERROR;
     }
 
-    digest_buffer = (unsigned char *)PORT_Alloc(hash_length);
+    auto digest_buffer = std::vector<unsigned char>(hash_length);
     unsigned int result_len = 0;
 
-    HASH_End(hash_context.get(), digest_buffer, &result_len, hash_length);
+    HASH_End(hash_context.get(), digest_buffer.data(), &result_len, digest_buffer.size());
 
     SECItem digest;
-    digest.data = digest_buffer;
-    digest.len = hash_length;
+    digest.data = digest_buffer.data();
+    digest.len = digest_buffer.size();
 
     if ((NSS_CMSSignerInfo_GetSigningCertificate(CMSSignerInfo, CERT_GetDefaultCertDB())) == nullptr) {
         CMSSignerInfo->verificationStatus = NSSCMSVS_SigningCertNotFound;
@@ -970,20 +969,15 @@ SignatureValidationStatus SignatureHandler::validateSignature()
           This means it's not a detached type signature
           so the digest is contained in SignedData->contentInfo
         */
-        if (memcmp(digest.data, content_info_data->data, hash_length) == 0 && digest.len == content_info_data->len) {
-            PORT_Free(digest_buffer);
+        if (digest.len == content_info_data->len && memcmp(digest.data, content_info_data->data, digest.len) == 0) {
             return SIGNATURE_VALID;
         } else {
-            PORT_Free(digest_buffer);
             return SIGNATURE_DIGEST_MISMATCH;
         }
 
     } else if (NSS_CMSSignerInfo_Verify(CMSSignerInfo, &digest, nullptr) != SECSuccess) {
-
-        PORT_Free(digest_buffer);
         return NSS_SigTranslate(CMSSignerInfo->verificationStatus);
     } else {
-        PORT_Free(digest_buffer);
         return SIGNATURE_VALID;
     }
 }