Fix uninitialize free, use finer sanity checks in auth. parser

Chenyangming9 · Mar 9, 2022 · 4a6291b · 4a6291b
1 parent 071852b
commit 4a6291b
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 9 deletions.
diff --git a/deps/authenticode-parser/src/authenticode.c b/deps/authenticode-parser/src/authenticode.c
@@ -561,14 +561,14 @@ AuthenticodeArray* parse_authenticode(const uint8_t* pe_data, long pe_len)
     uint64_t cert_len = letoh32(*(uint32_t*)(pe_data + pe_cert_table_addr + 4));
 
     /* we need atleast 8 bytes to read dwLength, revision and certType */
-    if (cert_len < 8 || pe_len < cert_addr + cert_len)
+    if (cert_len < 8 || pe_len < cert_addr + 8)
         return NULL;
 
     uint32_t dwLength = letoh32(*(uint32_t*)(pe_data + cert_addr));
     if (pe_len < cert_addr + dwLength)
         return NULL;
-
-    AuthenticodeArray* auth_array = authenticode_new(pe_data + cert_addr + 0x8, dwLength);
+    /* dwLength = offsetof(WIN_CERTIFICATE, bCertificate) + (size of the variable-length binary array contained within bCertificate) */
+    AuthenticodeArray* auth_array = authenticode_new(pe_data + cert_addr + 0x8, dwLength - 0x8);
     if (!auth_array)
         return NULL;
 

diff --git a/deps/authenticode-parser/src/countersignature.c b/deps/authenticode-parser/src/countersignature.c
@@ -133,16 +133,17 @@ Countersignature* pkcs9_countersig_new(
 
     /* compare the encrypted digest and calculated digest */
     bool isValid = false;
-    /* Sometimes signed data contains DER encoded DigestInfo structure which contains hash of
-     * authenticated attributes (39c9d136f026a9ad18fb9f41a64f76dd8418e8de625dce5d3a372bd242fc5edd)
-     * but other times it is just purely and I didn't find another way to  distinguish it but only
-     * based on the length of data we get. Found mention of this in openssl mailing list:
-     * https://mta.openssl.org/pipermail/openssl-users/2015-September/002054.html */
+
 #if OPENSSL_VERSION_NUMBER >= 0x3000000fL
     size_t mdLen = EVP_MD_get_size(md);
 #else
     size_t mdLen = EVP_MD_size(md);
 #endif
+    /* Sometimes signed data contains DER encoded DigestInfo structure which contains hash of
+     * authenticated attributes (39c9d136f026a9ad18fb9f41a64f76dd8418e8de625dce5d3a372bd242fc5edd)
+     * but other times it is just purely and I didn't find another way to  distinguish it but only
+     * based on the length of data we get. Found mention of this in openssl mailing list:
+     * https://mta.openssl.org/pipermail/openssl-users/2015-September/002054.html */
     if (mdLen == decLen) {
         isValid = !memcmp(calc_digest, decData, mdLen);
     } else {
@@ -199,7 +200,9 @@ Countersignature* ms_countersig_new(const uint8_t* data, long size, ASN1_STRING*
     const ASN1_TIME* rawTime = TS_TST_INFO_get_time(ts);
     if (!rawTime) {
         result->verify_flags = COUNTERSIGNATURE_VFY_TIME_MISSING;
-        goto end;
+        TS_TST_INFO_free(ts);
+        PKCS7_free(p7);
+        return result;
     }
 
     result->sign_time = ASN1_TIME_to_time_t(rawTime);