From 23487a45327dd54e46cd9f51bbec4303175e0976 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Tue, 2 Nov 2021 11:31:22 +0100 Subject: [PATCH 1/2] Fix a heap buffer overflow with mismatched PEM structure ZD13097 --- src/ssl.c | 6 ++++-- tests/api.c | 12 ++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 7a5975e26..7e9cb178c 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -43737,7 +43737,7 @@ err: } /* Read the header and footer */ - while (wolfSSL_BIO_read(bio, &pem[i], 1) == 1) { + while (i < l && wolfSSL_BIO_read(bio, &pem[i], 1) == 1) { i++; if (!header) { header = XSTRNSTR(pem, "-----BEGIN ", (unsigned int)i); @@ -43769,7 +43769,9 @@ err: if (footerEnd) { footerEnd += XSTR_SIZEOF("-----"); /* Now check that footer matches header */ - if (XMEMCMP(header + XSTR_SIZEOF("-----BEGIN "), + if ((headerEnd - (header + XSTR_SIZEOF("-----BEGIN "))) == + (footerEnd - (footer + XSTR_SIZEOF("-----END "))) && + XMEMCMP(header + XSTR_SIZEOF("-----BEGIN "), footer + XSTR_SIZEOF("-----END "), headerEnd - (header + XSTR_SIZEOF("-----BEGIN "))) != 0) { diff --git a/tests/api.c b/tests/api.c index 81aa480b1..0b3297958 100644 --- a/tests/api.c +++ b/tests/api.c @@ -29683,6 +29683,11 @@ static void test_wolfSSL_X509_INFO(void) X509_INFO *info; BIO *cert; int i; + byte data[] = { + "---------BEGIN CERTc-----\n" + "MIIDMTBuQ=\n" + "-----END -----" + }; printf(testingFmt, "wolfSSL_X509_INFO"); @@ -29701,6 +29706,13 @@ static void test_wolfSSL_X509_INFO(void) sk_X509_INFO_free(info_stack); BIO_free(cert); + /* This case should fail due to invalid input. */ + AssertNotNull(cert = BIO_new(BIO_s_mem())); + AssertIntEQ(BIO_write(cert, data, sizeof(data)), sizeof(data)); + AssertNull(info_stack = PEM_X509_INFO_read_bio(cert, NULL, NULL, NULL)); + sk_X509_INFO_pop_free(info_stack, X509_INFO_free); + BIO_free(cert); + printf(resultFmt, passed); #endif } From 1faa9e66b6e0be8e0149b18b0acf03414f1e5d45 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Thu, 4 Nov 2021 15:34:33 +0100 Subject: [PATCH 2/2] Check `wolfSSL_BIO_read` return --- src/ssl.c | 5 ++++- tests/api.c | 24 +++++++++++++++++++++--- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 7e9cb178c..fe7067cae 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -43750,7 +43750,10 @@ err: if (headerEnd) { headerEnd += XSTR_SIZEOF("-----"); /* Read in the newline */ - (void)wolfSSL_BIO_read(bio, &pem[i], 1); + if (wolfSSL_BIO_read(bio, &pem[i], 1) != 1) { + WOLFSSL_MSG("wolfSSL_BIO_read error"); + goto err; + } i++; if (*headerEnd != '\n' && *headerEnd != '\r') { WOLFSSL_MSG("Missing newline after header"); diff --git a/tests/api.c b/tests/api.c index 0b3297958..a8e315ce9 100644 --- a/tests/api.c +++ b/tests/api.c @@ -29683,10 +29683,23 @@ static void test_wolfSSL_X509_INFO(void) X509_INFO *info; BIO *cert; int i; + /* PEM in hex format to avoid null terminator */ byte data[] = { - "---------BEGIN CERTc-----\n" - "MIIDMTBuQ=\n" - "-----END -----" + 0x2d, 0x2d, 0x2d, 0x2d, 0x2d, 0x2d, 0x2d, 0x2d, 0x2d, 0x42, 0x45, 0x47, + 0x49, 0x4e, 0x20, 0x43, 0x45, 0x52, 0x54, 0x63, 0x2d, 0x2d, 0x2d, 0x2d, + 0x2d, 0x0a, 0x4d, 0x49, 0x49, 0x44, 0x4d, 0x54, 0x42, 0x75, 0x51, 0x3d, + 0x0a, 0x2d, 0x2d, 0x2d, 0x2d, 0x2d, 0x45, 0x4e, 0x44, 0x20, 0x2d, 0x2d, + 0x2d, 0x2d, 0x2d + }; + /* PEM in hex format to avoid null terminator */ + byte data2[] = { + 0x41, 0x53, 0x4e, 0x31, 0x20, 0x4f, 0x49, 0x44, 0x3a, 0x20, 0x70, 0x72, + 0x69, 0x6d, 0x65, 0x32, 0x35, 0x36, 0x76, 0x31, 0x0a, 0x2d, 0x2d, 0x2d, + 0x2d, 0x2d, 0x42, 0x45, 0x47, 0x49, 0x4e, 0x20, 0x45, 0x43, 0x20, 0x50, + 0x41, 0x52, 0x41, 0x4d, 0x45, 0x54, 0x45, 0x52, 0x53, 0x2d, 0x2d, 0x2d, + 0x2d, 0x43, 0x65, 0x72, 0x74, 0x69, 0x2d, 0x0a, 0x42, 0x67, 0x67, 0x71, + 0x68, 0x6b, 0x6a, 0x4f, 0x50, 0x51, 0x4d, 0x42, 0x42, 0x77, 0x3d, 0x3d, + 0x0a, 0x2d, 0x2d, 0x2d, 0x2d, 0x2d }; printf(testingFmt, "wolfSSL_X509_INFO"); @@ -29712,6 +29725,11 @@ static void test_wolfSSL_X509_INFO(void) AssertNull(info_stack = PEM_X509_INFO_read_bio(cert, NULL, NULL, NULL)); sk_X509_INFO_pop_free(info_stack, X509_INFO_free); BIO_free(cert); + AssertNotNull(cert = BIO_new(BIO_s_mem())); + AssertIntEQ(BIO_write(cert, data2, sizeof(data2)), sizeof(data2)); + AssertNull(info_stack = PEM_X509_INFO_read_bio(cert, NULL, NULL, NULL)); + sk_X509_INFO_pop_free(info_stack, X509_INFO_free); + BIO_free(cert); printf(resultFmt, passed); #endif