diff --git a/cab.c b/cab.c index 63e4f17..257c2f0 100644 --- a/cab.c +++ b/cab.c @@ -342,7 +342,8 @@ static int cab_verify_digests(FILE_FORMAT_CTX *ctx, PKCS7 *p7) const u_char *p = content_val->data; SpcIndirectDataContent *idc = d2i_SpcIndirectDataContent(NULL, &p, content_val->length); if (idc) { - if (spc_extract_digest_safe(idc, mdbuf, &mdtype) < 0) { + if (spc_indirect_data_content_get_digest(idc, mdbuf, &mdtype) < 0) { + fprintf(stderr, "Failed to extract message digest from signature\n\n"); SpcIndirectDataContent_free(idc); return 0; /* FAILED */ } diff --git a/cat.c b/cat.c index e46cdf6..8967d77 100644 --- a/cat.c +++ b/cat.c @@ -390,15 +390,12 @@ static int cat_print_content_member_digest(ASN1_TYPE *content) idc = d2i_SpcIndirectDataContent(NULL, &data, ASN1_STRING_length(value)); if (!idc) return 0; /* FAILED */ - if (spc_extract_digest_safe(idc, mdbuf, &mdtype) < 0) { + if (spc_indirect_data_content_get_digest(idc, mdbuf, &mdtype) < 0) { + fprintf(stderr, "Failed to extract message digest from signature\n\n"); SpcIndirectDataContent_free(idc); return 0; /* FAILED */ } SpcIndirectDataContent_free(idc); - if (mdtype == -1) { - fprintf(stderr, "Failed to extract current message digest\n\n"); - return 0; /* FAILED */ - } printf("\tHash algorithm: %s\n", OBJ_nid2sn(mdtype)); print_hash("\tMessage digest", "", mdbuf, EVP_MD_size(EVP_get_digestbynid(mdtype))); return 1; /* OK */ diff --git a/helpers.c b/helpers.c index e68b025..cbcbc38 100644 --- a/helpers.c +++ b/helpers.c @@ -548,33 +548,6 @@ SpcLink *spc_link_obsolete_get(void) return link; } -/* - * Safely extract digest from SpcIndirectDataContent - * [in] idc: parsed SpcIndirectDataContent - * [out] mdbuf: output buffer (must be EVP_MAX_MD_SIZE bytes) - * [out] mdtype: digest algorithm's NID - * [returns] -1 on error or digest length on success - */ -int spc_extract_digest_safe(SpcIndirectDataContent *idc, - u_char *mdbuf, int *mdtype) -{ - int digest_len; - - if (!idc || !idc->messageDigest || !idc->messageDigest->digest || - !idc->messageDigest->digestAlgorithm) { - fprintf(stderr, "Missing digest data\n"); - return -1; - } - digest_len = idc->messageDigest->digest->length; - if (digest_len <= 0 || digest_len > EVP_MAX_MD_SIZE) { - fprintf(stderr, "Invalid digest length: %d\n", digest_len); - return -1; - } - memcpy(mdbuf, idc->messageDigest->digest->data, (size_t)digest_len); - *mdtype = OBJ_obj2nid(idc->messageDigest->digestAlgorithm->algorithm); - return digest_len; -} - /* * [in] mdbuf, cmdbuf: message digests * [in] mdtype: message digest algorithm type @@ -590,6 +563,37 @@ int compare_digests(u_char *mdbuf, u_char *cmdbuf, int mdtype) return mdok; } +/* + * Safely extract digest from SpcIndirectDataContent with bounds checking. + * This function validates that the digest length from the ASN.1 structure + * does not exceed the destination buffer size, preventing buffer overflows + * from maliciously crafted signatures. + * [in] idc: parsed SpcIndirectDataContent structure + * [out] mdbuf: output buffer (must be at least EVP_MAX_MD_SIZE bytes) + * [out] mdtype: digest algorithm NID + * [returns] digest length on success, -1 on error + */ +int spc_indirect_data_content_get_digest(SpcIndirectDataContent *idc, u_char *mdbuf, int *mdtype) +{ + int digest_len; + + if (!idc || !idc->messageDigest || !idc->messageDigest->digest || + !idc->messageDigest->digestAlgorithm) { + return -1; /* FAILED */ + } + digest_len = idc->messageDigest->digest->length; + + /* Validate digest length to prevent buffer overflow */ + if (digest_len <= 0 || digest_len > EVP_MAX_MD_SIZE) { + fprintf(stderr, "Invalid digest length in signature: %d (expected 1-%d)\n", + digest_len, EVP_MAX_MD_SIZE); + return -1; /* FAILED */ + } + *mdtype = OBJ_obj2nid(idc->messageDigest->digestAlgorithm->algorithm); + memcpy(mdbuf, idc->messageDigest->digest->data, (size_t)digest_len); + return digest_len; /* OK */ +} + /* * Helper functions */ @@ -645,6 +649,10 @@ static int spc_indirect_data_content_create(u_char **blob, int *len, FILE_FORMAT idc->data->value->type = V_ASN1_SEQUENCE; idc->data->value->value.sequence = ASN1_STRING_new(); idc->data->type = ctx->format->data_blob_get(&p, &l, ctx); + if (!idc->data->type) { + SpcIndirectDataContent_free(idc); + return 0; /* FAILED */ + } idc->data->value->value.sequence->data = p; idc->data->value->value.sequence->length = l; idc->messageDigest->digestAlgorithm->algorithm = OBJ_nid2obj(mdtype); diff --git a/helpers.h b/helpers.h index a1d33ee..adaf416 100644 --- a/helpers.h +++ b/helpers.h @@ -24,9 +24,8 @@ int is_content_type(PKCS7 *p7, const char *objid); MsCtlContent *ms_ctl_content_get(PKCS7 *p7); ASN1_TYPE *catalog_content_get(CatalogAuthAttr *attribute); SpcLink *spc_link_obsolete_get(void); -int spc_extract_digest_safe(SpcIndirectDataContent *idc, - u_char *mdbuf, int *mdtype); int compare_digests(u_char *mdbuf, u_char *cmdbuf, int mdtype); +int spc_indirect_data_content_get_digest(SpcIndirectDataContent *idc, u_char *mdbuf, int *mdtype); /* Local Variables: diff --git a/msi.c b/msi.c index 8000821..affa65c 100644 --- a/msi.c +++ b/msi.c @@ -419,7 +419,8 @@ static int msi_verify_digests(FILE_FORMAT_CTX *ctx, PKCS7 *p7) const u_char *p = content_val->data; SpcIndirectDataContent *idc = d2i_SpcIndirectDataContent(NULL, &p, content_val->length); if (idc) { - if (spc_extract_digest_safe(idc, mdbuf, &mdtype) < 0) { + if (spc_indirect_data_content_get_digest(idc, mdbuf, &mdtype) < 0) { + fprintf(stderr, "Failed to extract message digest from signature\n\n"); SpcIndirectDataContent_free(idc); return 0; /* FAILED */ } diff --git a/osslsigncode.c b/osslsigncode.c index 49f39b7..5c3ea32 100644 --- a/osslsigncode.c +++ b/osslsigncode.c @@ -3082,12 +3082,8 @@ static int verify_content_member_digest(FILE_FORMAT_CTX *ctx, ASN1_TYPE *content fprintf(stderr, "Failed to extract SpcIndirectDataContent data\n"); return 1; /* FAILED */ } - if (spc_extract_digest_safe(idc, mdbuf, &mdtype) < 0) { - SpcIndirectDataContent_free(idc); - return 1; /* FAILED */ - } - if (mdtype == -1) { - fprintf(stderr, "Failed to extract current message digest\n\n"); + if (spc_indirect_data_content_get_digest(idc, mdbuf, &mdtype) < 0) { + fprintf(stderr, "Failed to extract message digest from signature\n\n"); SpcIndirectDataContent_free(idc); return 1; /* FAILED */ } diff --git a/pe.c b/pe.c index b984089..4ed385b 100644 --- a/pe.c +++ b/pe.c @@ -255,7 +255,9 @@ static int pe_verify_digests(FILE_FORMAT_CTX *ctx, PKCS7 *p7) SpcIndirectDataContent_free(idc); return 0; /* FAILED */ } - if (spc_extract_digest_safe(idc, mdbuf, &mdtype) < 0) { + if (spc_indirect_data_content_get_digest(idc, mdbuf, &mdtype) < 0) { + fprintf(stderr, "Failed to extract message digest from signature\n\n"); + OPENSSL_free(ph); SpcIndirectDataContent_free(idc); return 0; /* FAILED */ } @@ -920,6 +922,8 @@ static u_char *pe_page_hash_calc(int *rphlen, FILE_FORMAT_CTX *ctx, int phtype) char *sections; const EVP_MD *md = EVP_get_digestbynid(phtype); BIO *bhash; + uint32_t filebound; + size_t pphlen_sz, sections_factor; /* NumberOfSections indicates the size of the section table, * which immediately follows the headers, can be up to 65535 under Vista and later */ @@ -961,8 +965,29 @@ static u_char *pe_page_hash_calc(int *rphlen, FILE_FORMAT_CTX *ctx, int phtype) fprintf(stderr, "Corrupted optional header size: 0x%08X\n", opthdr_size); return NULL; /* FAILED */ } + /* Validate that pagesize >= hdrsize to prevent integer underflow */ + if (pagesize < hdrsize) { + fprintf(stderr, "Page size (0x%08X) is smaller than header size (0x%08X)\n", + pagesize, hdrsize); + return NULL; /* FAILED */ + } pphlen = 4 + EVP_MD_size(md); - phlen = pphlen * (3 + (int)nsections + (int)(ctx->pe_ctx->fileend / pagesize)); + + /* Use size_t arithmetic and check for overflow */ + pphlen_sz = (size_t)pphlen; + sections_factor = 3 + (size_t)nsections + ((size_t)ctx->pe_ctx->fileend / pagesize); + + /* Check for multiplication overflow */ + if (sections_factor > SIZE_MAX / pphlen_sz) { + fprintf(stderr, "Page hash allocation size would overflow\n"); + return NULL; /* FAILED */ + } + phlen = (int)(pphlen_sz * sections_factor); + /* Sanity limit - page hash shouldn't exceed reasonable size (16 MB) */ + if (phlen < 0 || (size_t)phlen > SIZE_16M) { + fprintf(stderr, "Page hash size exceeds limit: %d\n", phlen); + return NULL; /* FAILED */ + } bhash = BIO_new(BIO_f_md()); #if defined(__GNUC__) @@ -1008,14 +1033,24 @@ static u_char *pe_page_hash_calc(int *rphlen, FILE_FORMAT_CTX *ctx, int phtype) BIO_gets(bhash, (char*)res + 4, EVP_MD_size(md)); BIO_free_all(bhash); sections = ctx->options->indata + ctx->pe_ctx->header_size + 24 + opthdr_size; + /* Determine the file boundary for section data validation */ + filebound = ctx->pe_ctx->sigpos ? ctx->pe_ctx->sigpos : ctx->pe_ctx->fileend; for (i=0; i= UINT32_MAX) { sections += 40; continue; } + /* Validate section bounds against file size to prevent OOB read */ + if (ro >= filebound || rs > filebound - ro) { + fprintf(stderr, "Section %d has invalid bounds: offset=0x%08X, size=0x%08X, fileend=0x%08X\n", + i, ro, rs, filebound); + OPENSSL_free(zeroes); + OPENSSL_free(res); + return NULL; /* FAILED */ + } for (l=0; loptions->verbose) { diff --git a/script.c b/script.c index 8c97fc6..457791b 100644 --- a/script.c +++ b/script.c @@ -294,7 +294,8 @@ static int script_verify_digests(FILE_FORMAT_CTX *ctx, PKCS7 *p7) const u_char *p = content_val->data; SpcIndirectDataContent *idc = d2i_SpcIndirectDataContent(NULL, &p, content_val->length); if (idc) { - if (spc_extract_digest_safe(idc, mdbuf, &mdtype) < 0) { + if (spc_indirect_data_content_get_digest(idc, mdbuf, &mdtype) < 0) { + fprintf(stderr, "Failed to extract message digest from signature\n\n"); SpcIndirectDataContent_free(idc); return 0; /* FAILED */ }