Fixed integer overflow, integer underflow and Out-of-Bounds Read

Author: opera-aklajn

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 */
             }

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 */

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);

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:

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 */
             }

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 */
     }

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<nsections; i++) {
-        /* Resource Table address and size */
+        /* SizeOfRawData and PointerToRawData from section header */
         rs = GET_UINT32_LE(sections + 16);
         ro = GET_UINT32_LE(sections + 20);
         if (rs == 0 || rs >= 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; l<rs; l+=pagesize, pi++) {
             PUT_UINT32_LE(ro + l, res + pi*pphlen);
             bhash = BIO_new(BIO_f_md());
@@ -1088,6 +1123,10 @@ static int pe_verify_page_hash(FILE_FORMAT_CTX *ctx, u_char *ph, int phlen, int
     if (!ph)
         return 1; /* OK */
     cph = pe_page_hash_calc(&cphlen, ctx, phtype);
+    if (!cph) {
+        fprintf(stderr, "Page hash verification failed: could not calculate page hash\n");
+        return 0; /* FAILED */
+    }
     mdok = (phlen == cphlen) && !memcmp(ph, cph, (size_t)phlen);
     printf("Page hash algorithm  : %s\n", OBJ_nid2sn(phtype));
     if (ctx->options->verbose) {

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 */
             }