From 42d586b999bdf39fe84dc0eba596970bce8137bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Milkovi=C4=8D?= Date: Sat, 2 Mar 2024 13:25:33 +0100 Subject: [PATCH] Updated authenticode parser to latest version (2024-03-02) 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. --- .../authenticode-parser/authenticode.h | 2 + .../pe/authenticode-parser/authenticode.c | 53 +--------- .../pe/authenticode-parser/certificate.c | 98 +++++++++++++++++++ .../pe/authenticode-parser/certificate.h | 4 + .../pe/authenticode-parser/countersignature.c | 14 +++ 5 files changed, 123 insertions(+), 48 deletions(-) diff --git a/libyara/include/authenticode-parser/authenticode.h b/libyara/include/authenticode-parser/authenticode.h index 88f96a3cd9..d1675d1562 100644 --- a/libyara/include/authenticode-parser/authenticode.h +++ b/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 { diff --git a/libyara/modules/pe/authenticode-parser/authenticode.c b/libyara/modules/pe/authenticode-parser/authenticode.c index 892bfda9e5..f385860afb 100644 --- a/libyara/modules/pe/authenticode-parser/authenticode.c +++ b/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); diff --git a/libyara/modules/pe/authenticode-parser/certificate.c b/libyara/modules/pe/authenticode-parser/certificate.c index cf688e77b2..fc754e4ea2 100644 --- a/libyara/modules/pe/authenticode-parser/certificate.c +++ b/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) { diff --git a/libyara/modules/pe/authenticode-parser/certificate.h b/libyara/modules/pe/authenticode-parser/certificate.h index aefb797c6a..c5054fc1dc 100644 --- a/libyara/modules/pe/authenticode-parser/certificate.h +++ b/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); diff --git a/libyara/modules/pe/authenticode-parser/countersignature.c b/libyara/modules/pe/authenticode-parser/countersignature.c index 26fd42d100..0fb45760fd 100644 --- a/libyara/modules/pe/authenticode-parser/countersignature.c +++ b/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); } }