commit ec81709516903a8590321eacdef0f6bf074e68f2
author Nick Child <nick.child@ibm.com> Thu Dec 15 15:54:03 2022 -0600
committer Nick Child <nick.child@ibm.com> Mon Jan 30 16:44:58 2023 +0000
tree a2d258ae3689ccba13f17a2849933be697dc6d92
parent 4983ddf74777304a1854f56db4bf33dbafcdd616

pkcs7: Ensure all data in asn1 structure is accounted for

Several PKCS7 invalid ASN1 Tests were failing due to extra
data bytes or incorrect content lengths going unnoticed. Make
the parser aware of possible malformed ASN1 data.

Signed-off-by: Nick Child <nick.child@ibm.com>

library/pkcs7.c

diff --git a/library/pkcs7.c b/library/pkcs7.c
index 5fd8f64..05d98c3 100644
--- a/library/pkcs7.c
+++ b/library/pkcs7.c
@@ -94,6 +94,7 @@
  *              [0] EXPLICIT ANY DEFINED BY contentType OPTIONAL }
  **/
 static int pkcs7_get_content_info_type(unsigned char **p, unsigned char *end,
+                                       unsigned char **seq_end,
                                        mbedtls_pkcs7_buf *pkcs7)
 {
     size_t len = 0;
@@ -106,8 +107,8 @@
         *p = start;
         return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO, ret);
     }
-
-    ret = mbedtls_asn1_get_tag(p, end, &len, MBEDTLS_ASN1_OID);
+    *seq_end = *p + len;
+    ret = mbedtls_asn1_get_tag(p, *seq_end, &len, MBEDTLS_ASN1_OID);
     if (ret != 0) {
         *p = start;
         return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO, ret);
@@ -289,7 +290,7 @@
 static int pkcs7_get_signer_info(unsigned char **p, unsigned char *end,
                                  mbedtls_pkcs7_signer_info *signer)
 {
-    unsigned char *end_signer;
+    unsigned char *end_signer, *end_issuer_and_sn;
     int asn1_ret = 0, ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
     size_t len = 0;
 
@@ -312,6 +313,7 @@
         goto out;
     }
 
+    end_issuer_and_sn = *p + len;
     /* Parsing IssuerAndSerialNumber */
     signer->issuer_raw.p = *p;
 
@@ -333,6 +335,12 @@
         goto out;
     }
 
+    /* ensure no extra or missing bytes */
+    if (*p != end_issuer_and_sn) {
+        ret = MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO;
+        goto out;
+    }
+
     ret = pkcs7_get_digest_algorithm(p, end_signer, &signer->alg_identifier);
     if (ret != 0) {
         goto out;
@@ -449,7 +457,7 @@
 {
     unsigned char *p = buf;
     unsigned char *end = buf + buflen;
-    unsigned char *end_set;
+    unsigned char *end_set, *end_content_info;
     size_t len = 0;
     int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
     mbedtls_md_type_t md_alg;
@@ -481,11 +489,16 @@
     }
 
     /* Do not expect any content */
-    ret = pkcs7_get_content_info_type(&p, end_set, &signed_data->content.oid);
+    ret = pkcs7_get_content_info_type(&p, end_set, &end_content_info,
+                                      &signed_data->content.oid);
     if (ret != 0) {
         return ret;
     }
 
+    if (end_content_info != p) {
+        return MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO;
+    }
+
     if (MBEDTLS_OID_CMP(MBEDTLS_OID_PKCS7_DATA, &signed_data->content.oid)) {
         return MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO;
     }
@@ -527,7 +540,7 @@
                             const size_t buflen)
 {
     unsigned char *p;
-    unsigned char *end;
+    unsigned char *end, *end_content_info;
     size_t len = 0;
     int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
     int isoidset = 0;
@@ -546,12 +559,19 @@
     pkcs7->raw.len = buflen;
     end = p + buflen;
 
-    ret = pkcs7_get_content_info_type(&p, end, &pkcs7->content_type_oid);
+    ret = pkcs7_get_content_info_type(&p, end, &end_content_info,
+                                      &pkcs7->content_type_oid);
     if (ret != 0) {
         len = buflen;
         goto try_data;
     }
 
+    /* Ensure PKCS7 data uses the exact number of bytes specified in buflen */
+    if (end_content_info != end) {
+        ret = MBEDTLS_ERR_PKCS7_BAD_INPUT_DATA;
+        goto out;
+    }
+
     if (!MBEDTLS_OID_CMP(MBEDTLS_OID_PKCS7_DATA, &pkcs7->content_type_oid)
         || !MBEDTLS_OID_CMP(MBEDTLS_OID_PKCS7_ENCRYPTED_DATA, &pkcs7->content_type_oid)
         || !MBEDTLS_OID_CMP(MBEDTLS_OID_PKCS7_ENVELOPED_DATA, &pkcs7->content_type_oid)
@@ -574,6 +594,12 @@
         goto out;
     }
 
+    /* ensure no extra/missing data */
+    if (p + len != end) {
+        ret = MBEDTLS_ERR_PKCS7_BAD_INPUT_DATA;
+        goto out;
+    }
+
 try_data:
     ret = pkcs7_get_signed_data(p, len, &pkcs7->signed_data);
     if (ret != 0) {