Updated authenticode parser to latest version (2024-03-02) (#2049)

Upon fixing the latest issue regarding parsing CMS, we figured out we
are not properly extracting all certificates from Microsoft
countersignatures. This should now properly expose them.

Also fixes one memory leak for countersignature parsing.

Author: metthal

libyara/include/authenticode-parser/authenticode.h

@@ -124,6 +124,8 @@ typedef struct {
     char* digest_alg;        /* Name of the digest algorithm used */
     ByteArray digest;        /* Stored message digest */
     CertificateArray* chain; /* Certificate chain of the signer */
+    CertificateArray* certs; /* All certs stored inside Countersignature, this can be superset
+                                of chain in case of non PKCS9 countersignature*/
 } Countersignature;
 
 typedef struct {

libyara/modules/pe/authenticode-parser/authenticode.c

@@ -114,23 +114,6 @@ static char* parse_program_name(ASN1_TYPE* spcAttr)
     return result;
 }
 
-/* Parses X509* certs into internal representation and inserts into CertificateArray
- * Array is assumed to have enough space to hold all certificates storted in the STACK */
-static void parse_certificates(const STACK_OF(X509) * certs, CertificateArray* result)
-{
-    int certCount = sk_X509_num(certs);
-    int i = 0;
-    for (; i < certCount; ++i) {
-        Certificate* cert = certificate_new(sk_X509_value(certs, i));
-        if (!cert)
-            break;
-
-        /* Write to the result */
-        result->certs[i] = cert;
-    }
-    result->count = i;
-}
-
 static void parse_nested_authenticode(PKCS7_SIGNER_INFO* si, AuthenticodeArray* result)
 {
     STACK_OF(X509_ATTRIBUTE)* attrs = PKCS7_get_attributes(si);
@@ -190,32 +173,6 @@ static void parse_pkcs9_countersig(PKCS7* p7, Authenticode* auth)
     }
 }
 
-/* Extracts X509 certificates from MS countersignature and stores them into result */
-static void extract_ms_counter_certs(const uint8_t* data, int len, CertificateArray* result)
-{
-    PKCS7* p7 = d2i_PKCS7(NULL, &data, len);
-    if (!p7)
-        return;
-
-    /* We expect SignedData type of PKCS7 */
-    if (!PKCS7_type_is_signed(p7) || !p7->d.sign) {
-        PKCS7_free(p7);
-        return;
-    }
-
-    STACK_OF(X509)* certs = p7->d.sign->cert;
-    CertificateArray* certArr = certificate_array_new(sk_X509_num(certs));
-    if (!certArr) {
-        PKCS7_free(p7);
-        return;
-    }
-    parse_certificates(certs, certArr);
-    certificate_array_move(result, certArr);
-    certificate_array_free(certArr);
-
-    PKCS7_free(p7);
-}
-
 static void parse_ms_countersig(PKCS7* p7, Authenticode* auth)
 {
     PKCS7_SIGNER_INFO* si = sk_PKCS7_SIGNER_INFO_value(PKCS7_get_signer_info(p7), 0);
@@ -239,14 +196,14 @@ static void parse_ms_countersig(PKCS7* p7, Authenticode* auth)
         int len = nested->value.sequence->length;
         const uint8_t* data = nested->value.sequence->data;
 
-        Countersignature* sig = ms_countersig_new(data, len, si->enc_digest);
-        if (!sig)
+        Countersignature* csig = ms_countersig_new(data, len, si->enc_digest);
+        if (!csig)
             return;
 
+        countersignature_array_insert(auth->countersigs, csig);
         /* Because MS TimeStamp countersignature has it's own SET of certificates
          * extract it back into parent signature for consistency with PKCS9 */
-        countersignature_array_insert(auth->countersigs, sig);
-        extract_ms_counter_certs(data, len, auth->certs);
+        certificate_array_append(auth->certs, csig->certs);
     }
 }
 
@@ -344,7 +301,7 @@ AuthenticodeArray* authenticode_new(const uint8_t* data, int32_t len)
         auth->verify_flags = AUTHENTICODE_VFY_INTERNAL_ERROR;
         goto end;
     }
-    parse_certificates(certs, auth->certs);
+    parse_x509_certificates(certs, auth->certs);
 
     /* Get Signature content that contains the message digest and it's algorithm */
     SpcIndirectDataContent* dataContent = get_content(p7data->contents);

libyara/modules/pe/authenticode-parser/certificate.c

@@ -330,10 +330,82 @@ Certificate* certificate_new(X509* x509)
     return result;
 }
 
+void attributes_copy(Attributes* dst, Attributes* src)
+{
+    byte_array_init(&dst->country, src->country.data, src->country.len);
+    byte_array_init(&dst->organization, src->organization.data, src->organization.len);
+    byte_array_init(
+        &dst->organizationalUnit, src->organizationalUnit.data, src->organizationalUnit.len);
+    byte_array_init(&dst->nameQualifier, src->nameQualifier.data, src->nameQualifier.len);
+    byte_array_init(&dst->state, src->state.data, src->state.len);
+    byte_array_init(&dst->commonName, src->commonName.data, src->commonName.len);
+    byte_array_init(&dst->serialNumber, src->serialNumber.data, src->serialNumber.len);
+    byte_array_init(&dst->locality, src->locality.data, src->locality.len);
+    byte_array_init(&dst->title, src->title.data, src->title.len);
+    byte_array_init(&dst->surname, src->surname.data, src->surname.len);
+    byte_array_init(&dst->givenName, src->givenName.data, src->givenName.len);
+    byte_array_init(&dst->initials, src->initials.data, src->initials.len);
+    byte_array_init(&dst->pseudonym, src->pseudonym.data, src->pseudonym.len);
+    byte_array_init(
+        &dst->generationQualifier, src->generationQualifier.data, src->generationQualifier.len);
+    byte_array_init(&dst->emailAddress, src->emailAddress.data, src->emailAddress.len);
+}
+
+/* Parses X509* certs into internal representation and inserts into CertificateArray
+ * Array is assumed to have enough space to hold all certificates storted in the STACK */
+void parse_x509_certificates(const STACK_OF(X509) * certs, CertificateArray* result)
+{
+    int certCount = sk_X509_num(certs);
+    int i = 0;
+    for (; i < certCount; ++i) {
+        Certificate* cert = certificate_new(sk_X509_value(certs, i));
+        if (!cert)
+            break;
+
+        /* Write to the result */
+        result->certs[i] = cert;
+    }
+    result->count = i;
+}
+
+/* Creates deep copy of a certificate */
+Certificate* certificate_copy(Certificate* cert)
+{
+    if (!cert)
+        return NULL;
+
+    Certificate* result = (Certificate*)calloc(1, sizeof(*result));
+    if (!result)
+        return NULL;
+
+    result->version = cert->version;
+    result->issuer = cert->issuer ? strdup(cert->issuer) : NULL;
+    result->subject = cert->subject ? strdup(cert->subject) : NULL;
+    result->serial = cert->serial ? strdup(cert->serial) : NULL;
+    result->not_after = cert->not_after;
+    result->not_before = cert->not_before;
+    result->sig_alg = cert->sig_alg ? strdup(cert->sig_alg) : NULL;
+    result->sig_alg_oid = cert->sig_alg_oid ? strdup(cert->sig_alg_oid) : NULL;
+    result->key_alg = cert->key_alg ? strdup(cert->key_alg) : NULL;
+    result->key = cert->key ? strdup(cert->key) : NULL;
+    byte_array_init(&result->sha1, cert->sha1.data, cert->sha1.len);
+    byte_array_init(&result->sha256, cert->sha256.data, cert->sha256.len);
+    attributes_copy(&result->issuer_attrs, &cert->issuer_attrs);
+    attributes_copy(&result->subject_attrs, &cert->subject_attrs);
+
+    return result;
+}
+
 /* Moves certificates from src to dst, returns 0 on success,
  * else 1. If error occurs, arguments are unchanged */
 int certificate_array_move(CertificateArray* dst, CertificateArray* src)
 {
+    if (!dst || !src)
+        return 1;
+
+    if (!src->certs || !src->count)
+        return 0;
+
     size_t newCount = dst->count + src->count;
 
     Certificate** tmp = (Certificate**)realloc(dst->certs, newCount * sizeof(Certificate*));
@@ -354,6 +426,32 @@ int certificate_array_move(CertificateArray* dst, CertificateArray* src)
     return 0;
 }
 
+/* Copies certificates from src and appends to dst, returns 0 on success,
+ * else 1. If error occurs, arguments are unchanged */
+int certificate_array_append(CertificateArray* dst, CertificateArray* src)
+{
+    if (!dst || !src)
+        return 1;
+
+    if (!src->certs || !src->count)
+        return 0;
+
+    size_t newCount = dst->count + src->count;
+
+    Certificate** tmp = (Certificate**)realloc(dst->certs, newCount * sizeof(Certificate*));
+    if (!tmp)
+        return 1;
+
+    dst->certs = tmp;
+
+    for (size_t i = 0; i < src->count; ++i)
+        dst->certs[i + dst->count] = certificate_copy(src->certs[i]);
+
+    dst->count = newCount;
+
+    return 0;
+}
+
 /* Allocates empty certificate array with reserved space for certCount certs */
 CertificateArray* certificate_array_new(int certCount)
 {

libyara/modules/pe/authenticode-parser/certificate.h

@@ -31,10 +31,14 @@ extern "C" {
 #endif
 
 Certificate* certificate_new(X509* x509);
+Certificate* certificate_copy(Certificate* cert);
 void certificate_free(Certificate* cert);
 
+void parse_x509_certificates(const STACK_OF(X509) * certs, CertificateArray* result);
+
 CertificateArray* parse_signer_chain(X509* signer_cert, STACK_OF(X509) * certs);
 int certificate_array_move(CertificateArray* dst, CertificateArray* src);
+int certificate_array_append(CertificateArray* dst, CertificateArray* src);
 CertificateArray* certificate_array_new(int certCount);
 void certificate_array_free(CertificateArray* arr);
 

libyara/modules/pe/authenticode-parser/countersignature.c

@@ -456,6 +456,9 @@ CountersignatureImpl* ms_countersig_impl_new(const uint8_t* data, long size)
         result->funcs = &FUNC_ARRAY_NAME_FOR_IMPL(pkcs7);
         result->pkcs7 = p7;
         return result;
+    } else if (p7) {
+        PKCS7_free(p7);
+        return NULL;
     }
 
     d = data;
@@ -526,6 +529,16 @@ Countersignature* ms_countersig_new(const uint8_t* data, long size, ASN1_STRING*
     }
 
     STACK_OF(X509)* certs = impl->funcs->get_certs(impl);
+
+    /* MS Counter signatures (PKCS7/CMS) can have extra certificates that are not part of a chain */
+    result->certs = certificate_array_new(sk_X509_num(certs));
+    if (!result->certs) {
+        result->verify_flags = AUTHENTICODE_VFY_INTERNAL_ERROR;
+        goto end;
+    }
+
+    parse_x509_certificates(certs, result->certs);
+
     result->chain = parse_signer_chain(signCert, certs);
 
     /* Imprint == digest */
@@ -641,6 +654,7 @@ void countersignature_free(Countersignature* sig)
         free(sig->digest_alg);
         free(sig->digest.data);
         certificate_array_free(sig->chain);
+        certificate_array_free(sig->certs);
         free(sig);
     }
 }