diff --git a/include/mbedtls/pkcs7.h b/include/mbedtls/pkcs7.h index 835d2c1a5..b2cdabcfd 100644 --- a/include/mbedtls/pkcs7.h +++ b/include/mbedtls/pkcs7.h @@ -179,8 +179,9 @@ void mbedtls_pkcs7_init(mbedtls_pkcs7 *pkcs7); * \brief Parse a single DER formatted pkcs7 content. * * \param pkcs7 The pkcs7 structure to be filled by parser for the output. - * \param buf The buffer holding the DER encoded pkcs7. - * \param buflen The size in bytes of \p buf. + * \param buf The buffer holding only the DER encoded pkcs7. + * \param buflen The size in bytes of \p buf. The size must be exactly the + * length of the DER encoded pkcs7. * * \note This function makes an internal copy of the PKCS7 buffer * \p buf. In particular, \p buf may be destroyed or reused diff --git a/library/pkcs7.c b/library/pkcs7.c index 5fd8f6444..05d98c353 100644 --- a/library/pkcs7.c +++ b/library/pkcs7.c @@ -94,6 +94,7 @@ static int pkcs7_get_version(unsigned char **p, unsigned char *end, int *ver) * [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 @@ static int pkcs7_get_content_info_type(unsigned char **p, unsigned char *end, *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 void pkcs7_free_signer_info(mbedtls_pkcs7_signer_info *signer) 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 @@ static int pkcs7_get_signer_info(unsigned char **p, unsigned char *end, goto out; } + end_issuer_and_sn = *p + len; /* Parsing IssuerAndSerialNumber */ signer->issuer_raw.p = *p; @@ -333,6 +335,12 @@ static int pkcs7_get_signer_info(unsigned char **p, unsigned char *end, 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 @@ static int pkcs7_get_signed_data(unsigned char *buf, size_t buflen, { 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 @@ static int pkcs7_get_signed_data(unsigned char *buf, size_t buflen, } /* 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 @@ int mbedtls_pkcs7_parse_der(mbedtls_pkcs7 *pkcs7, const unsigned char *buf, 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 @@ int mbedtls_pkcs7_parse_der(mbedtls_pkcs7 *pkcs7, const unsigned char *buf, 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 @@ int mbedtls_pkcs7_parse_der(mbedtls_pkcs7 *pkcs7, const unsigned char *buf, 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) { diff --git a/tests/suites/test_suite_pkcs7.data b/tests/suites/test_suite_pkcs7.data index c916e422b..22dd8a446 100644 --- a/tests/suites/test_suite_pkcs7.data +++ b/tests/suites/test_suite_pkcs7.data @@ -64,11 +64,11 @@ pkcs7_parse:"data_files/pkcs7_signerInfo_serial_invalid_size.der":MBEDTLS_ERR_PK pkcs7_get_signers_info_set error handling (6213931373035520) depends_on:MBEDTLS_RIPEMD160_C -pkcs7_parse:"data_files/pkcs7_get_signers_info_set-missing_free-fuzz_pkcs7-6213931373035520.der":MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG +pkcs7_parse:"data_files/pkcs7_get_signers_info_set-missing_free-fuzz_pkcs7-6213931373035520.der":MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO pkcs7_get_signers_info_set error handling (4541044530479104) depends_on:MBEDTLS_RIPEMD160_C -pkcs7_parse:"data_files/pkcs7_get_signers_info_set-leak-fuzz_pkcs7-4541044530479104.der":MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO +pkcs7_parse:"data_files/pkcs7_get_signers_info_set-leak-fuzz_pkcs7-4541044530479104.der":MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO PKCS7 Only Signed Data Parse Pass #15 depends_on:MBEDTLS_SHA256_C:MBEDTLS_RSA_C