From 1362c5ab16015c6510a10a90ee2b4839e1ed39c9 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Tue, 29 Nov 2022 17:16:55 -0500 Subject: [PATCH 1/2] Test for both PKCS 7 bugs found by OSS-Fuzz MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously the same test was repeated twice. Signed-off-by: Demi Marie Obenour Signed-off-by: Bence Szépkúti --- tests/suites/test_suite_pkcs7.data | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_pkcs7.data b/tests/suites/test_suite_pkcs7.data index f3cbb628f..571d5adf4 100644 --- a/tests/suites/test_suite_pkcs7.data +++ b/tests/suites/test_suite_pkcs7.data @@ -68,7 +68,7 @@ pkcs7_parse:"data_files/pkcs7_get_signers_info_set-missing_free-fuzz_pkcs7-62139 pkcs7_get_signers_info_set error handling (4541044530479104) 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-leak-fuzz_pkcs7-4541044530479104.der":MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO PKCS7 Only Signed Data Parse Pass #15 depends_on:MBEDTLS_SHA256_C:MBEDTLS_RSA_C From f7641544eafeaf0c71d109fbbec1d9f8aa2e74d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bence=20Sz=C3=A9pk=C3=BAti?= Date: Mon, 12 Dec 2022 21:37:36 +0100 Subject: [PATCH 2/2] Correct the fix for the PKCS 7 memory leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This corrects an issue in the origina fix in 4f01121f6e598c51e42a69f3fd9a54846013117a. Signed-off-by: Bence Szépkúti --- library/pkcs7.c | 45 +++++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/library/pkcs7.c b/library/pkcs7.c index 5b22afac9..9100980f6 100644 --- a/library/pkcs7.c +++ b/library/pkcs7.c @@ -253,6 +253,24 @@ static int pkcs7_get_signature( unsigned char **p, unsigned char *end, return( 0 ); } +static void pkcs7_free_signer_info( mbedtls_pkcs7_signer_info *signer ) +{ + mbedtls_x509_name *name_cur; + mbedtls_x509_name *name_prv; + + if( signer == NULL ) + return; + + name_cur = signer->issuer.next; + while( name_cur != NULL ) + { + name_prv = name_cur; + name_cur = name_cur->next; + mbedtls_free( name_prv ); + } + signer->issuer.next = NULL; +} + /** * SignerInfo ::= SEQUENCE { * version Version; @@ -329,33 +347,16 @@ static int pkcs7_get_signer_info( unsigned char **p, unsigned char *end, ret = MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO; out: - if( asn1_ret != 0 ) + if( asn1_ret != 0 || ret != 0 ) + { + pkcs7_free_signer_info( signer ); ret = MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO, asn1_ret ); - else if( ret != 0 ) - ret = MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO; + } return( ret ); } -static void pkcs7_free_signer_info( mbedtls_pkcs7_signer_info *signer ) -{ - mbedtls_x509_name *name_cur; - mbedtls_x509_name *name_prv; - - if( signer == NULL ) - return; - - name_cur = signer->issuer.next; - while( name_cur != NULL ) - { - name_prv = name_cur; - name_cur = name_cur->next; - mbedtls_free( name_prv ); - } - signer->issuer.next = NULL; -} - /** * SignerInfos ::= SET of SignerInfo * Return number of signers added to the signed data, @@ -387,7 +388,7 @@ static int pkcs7_get_signers_info_set( unsigned char **p, unsigned char *end, ret = pkcs7_get_signer_info( p, end_set, signers_set ); if( ret != 0 ) - goto cleanup; + return( ret ); count++; mbedtls_pkcs7_signer_info *prev = signers_set;