From 63bcf189be73a9cc1264059bed6f57974be74a83 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 13 Dec 2022 14:54:55 +0000 Subject: [PATCH 04/18] Avoid dangling ptrs in header and data params for PEM_read_bio_ex In the event of a failure in PEM_read_bio_ex() we free the buffers we allocated for the header and data buffers. However we were not clearing the ptrs stored in *header and *data. Since, on success, the caller is responsible for freeing these ptrs this can potentially lead to a double free if the caller frees them even on failure. Thanks to Dawei Wang for reporting this issue. Based on a proposed patch by Kurt Roeckx. CVE-2022-4450 Reviewed-by: Paul Dale Reviewed-by: Hugo Landau --- crypto/pem/pem_lib.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crypto/pem/pem_lib.c b/crypto/pem/pem_lib.c index f9ff80162a..85c47fb627 100644 --- a/crypto/pem/pem_lib.c +++ b/crypto/pem/pem_lib.c @@ -989,7 +989,9 @@ int PEM_read_bio_ex(BIO *bp, char **name_out, char **header, *data = pem_malloc(len, flags); if (*header == NULL || *data == NULL) { pem_free(*header, flags, 0); + *header = NULL; pem_free(*data, flags, 0); + *data = NULL; goto end; } BIO_read(headerB, *header, headerlen); -- 2.39.1 From cbafa34b5a057794c5c08cd4657038e1f643c1ac Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 13 Dec 2022 15:02:26 +0000 Subject: [PATCH 05/18] Add a test for CVE-2022-4450 Call PEM_read_bio_ex() and expect a failure. There should be no dangling ptrs and therefore there should be no double free if we free the ptrs on error. Reviewed-by: Paul Dale Reviewed-by: Hugo Landau --- test/pemtest.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/test/pemtest.c b/test/pemtest.c index a8d2d49bb5..a5d28cb256 100644 --- a/test/pemtest.c +++ b/test/pemtest.c @@ -96,6 +96,35 @@ static int test_cert_key_cert(void) return 1; } +static int test_empty_payload(void) +{ + BIO *b; + static char *emptypay = + "-----BEGIN CERTIFICATE-----\n" + "-\n" /* Base64 EOF character */ + "-----END CERTIFICATE-----"; + char *name = NULL, *header = NULL; + unsigned char *data = NULL; + long len; + int ret = 0; + + b = BIO_new_mem_buf(emptypay, strlen(emptypay)); + if (!TEST_ptr(b)) + return 0; + + /* Expected to fail because the payload is empty */ + if (!TEST_false(PEM_read_bio_ex(b, &name, &header, &data, &len, 0))) + goto err; + + ret = 1; + err: + OPENSSL_free(name); + OPENSSL_free(header); + OPENSSL_free(data); + BIO_free(b); + return ret; +} + int setup_tests(void) { if (!TEST_ptr(pemfile = test_get_argument(0))) @@ -103,5 +132,6 @@ int setup_tests(void) ADD_ALL_TESTS(test_b64, OSSL_NELEM(b64_pem_data)); ADD_TEST(test_invalid); ADD_TEST(test_cert_key_cert); + ADD_TEST(test_empty_payload); return 1; } -- 2.39.1