diff --git a/ChangeLog.d/pkcs-free-stale-pointers.txt b/ChangeLog.d/pkcs-free-stale-pointers.txt new file mode 100644 index 0000000000..2c29fed5ef --- /dev/null +++ b/ChangeLog.d/pkcs-free-stale-pointers.txt @@ -0,0 +1,6 @@ +Security + * Fix a use-after-free/double-free risk in mbedtls_pkcs7_free() when reusing + an mbedtls_pkcs7 context across parse -> free -> parse -> free cycles. The + function now resets the context after freeing it, ensuring stale + signed_data.signers.next pointers cannot be walked by a later free + operation. CVE-2026-50579 diff --git a/library/pkcs7.c b/library/pkcs7.c index 2cc7812bf0..e345d4b980 100644 --- a/library/pkcs7.c +++ b/library/pkcs7.c @@ -766,7 +766,7 @@ void mbedtls_pkcs7_free(mbedtls_pkcs7 *pkcs7) mbedtls_free(signer_prev); } - pkcs7->raw.p = NULL; + mbedtls_platform_zeroize(pkcs7, sizeof(*pkcs7)); } #endif diff --git a/tests/suites/test_suite_pkcs7.data b/tests/suites/test_suite_pkcs7.data index 3e3f7f1d7d..57f71841e2 100644 --- a/tests/suites/test_suite_pkcs7.data +++ b/tests/suites/test_suite_pkcs7.data @@ -14,6 +14,14 @@ PKCS7 Signed Data Parse with zero signers depends_on:PSA_WANT_ALG_SHA_256 pkcs7_parse:"../framework/data_files/pkcs7_data_no_signers.der":MBEDTLS_PKCS7_SIGNED_DATA +PKCS7 Signed Data Parse reused object after multiple signers +depends_on:PSA_WANT_ALG_SHA_256:PSA_WANT_KEY_TYPE_RSA_PUBLIC_KEY +pkcs7_parse_reuse:"../framework/data_files/pkcs7_data_multiple_signed.der":"../framework/data_files/pkcs7_data_no_signers.der":MBEDTLS_PKCS7_SIGNED_DATA + +PKCS7 Signed Data Parse reused object from multiple signers to one signer +depends_on:PSA_WANT_ALG_SHA_256:PSA_WANT_KEY_TYPE_RSA_PUBLIC_KEY +pkcs7_parse_reuse:"../framework/data_files/pkcs7_data_multiple_signed.der":"../framework/data_files/pkcs7_data_cert_signed_sha256.der":MBEDTLS_PKCS7_SIGNED_DATA + PKCS7 Signed Data Parse Fail with multiple certs #4 depends_on:PSA_WANT_ALG_SHA_256:PSA_WANT_KEY_TYPE_RSA_PUBLIC_KEY pkcs7_parse:"../framework/data_files/pkcs7_data_multiple_certs_signed.der":MBEDTLS_ERR_PKCS7_FEATURE_UNAVAILABLE diff --git a/tests/suites/test_suite_pkcs7.function b/tests/suites/test_suite_pkcs7.function index 9eccabab22..3bd7595b96 100644 --- a/tests/suites/test_suite_pkcs7.function +++ b/tests/suites/test_suite_pkcs7.function @@ -25,8 +25,10 @@ static int pkcs7_parse_buffer(unsigned char *pkcs7_buf, int buflen) mbedtls_pkcs7_init(&pkcs7); res = mbedtls_pkcs7_parse_der(&pkcs7, pkcs7_buf, buflen); mbedtls_pkcs7_free(&pkcs7); + return res; } + /* END_SUITE_HELPERS */ /* BEGIN_CASE */ @@ -71,6 +73,48 @@ exit: } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_FS_IO */ +void pkcs7_parse_reuse(char *first_pkcs7_file, char *second_pkcs7_file, + int res_expect) +{ + unsigned char *first_pkcs7_buf = NULL; + unsigned char *second_pkcs7_buf = NULL; + mbedtls_pkcs7 pkcs7; + size_t first_buflen; + size_t second_buflen; + + mbedtls_pkcs7_init(&pkcs7); + + /* PKCS7 uses X509 which itself relies on PK under the hood and the latter + * can use PSA to store keys and perform operations so psa_crypto_init() + * must be called before. */ + USE_PSA_INIT(); + + TEST_EQUAL(mbedtls_pk_load_file(first_pkcs7_file, &first_pkcs7_buf, + &first_buflen), 0); + + TEST_EQUAL(mbedtls_pk_load_file(second_pkcs7_file, &second_pkcs7_buf, + &second_buflen), 0); + + TEST_EQUAL(mbedtls_pkcs7_parse_der(&pkcs7, first_pkcs7_buf, + first_buflen), + MBEDTLS_PKCS7_SIGNED_DATA); + + mbedtls_pkcs7_free(&pkcs7); + TEST_ASSERT(pkcs7.raw.p == NULL); + TEST_ASSERT(pkcs7.signed_data.signers.next == NULL); + TEST_EQUAL(mbedtls_pkcs7_parse_der(&pkcs7, second_pkcs7_buf, + second_buflen), + res_expect); + +exit: + mbedtls_free(first_pkcs7_buf); + mbedtls_free(second_pkcs7_buf); + mbedtls_pkcs7_free(&pkcs7); + USE_PSA_DONE(); +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_FS_IO:MBEDTLS_X509_CRT_PARSE_C:PSA_HAVE_ALG_SOME_RSA_VERIFY */ void pkcs7_verify(char *pkcs7_file, char *crt_files,