From 3468750bae2491296a3a316fb71cabd6128113bc Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Thu, 6 Apr 2023 14:31:53 -0500 Subject: [PATCH 1/2] wolfcrypt/src/asn.c: * refactor error-checking cascade in wc_PemCertToDer_ex() as in wc_PemPubKeyToDer_ex(), * refactor staticBuffer gating/dynamics in wc_PemPubKeyToDer_ex() as in wc_PemCertToDer_ex(), * and use IO_FAILED_E, not BUFFER_E, for I/O errors on the file handles, in both routines; fix smallstack null pointer dereferences in src/pk.c:wolfSSL_RSA_GenAdd() and src/ssl.c:set_curves_list(). --- src/pk.c | 21 ++++++---- src/ssl.c | 2 +- wolfcrypt/src/asn.c | 97 +++++++++++++++++++++++++-------------------- 3 files changed, 68 insertions(+), 52 deletions(-) diff --git a/src/pk.c b/src/pk.c index e8be52921..75e239fec 100644 --- a/src/pk.c +++ b/src/pk.c @@ -4456,12 +4456,7 @@ int wolfSSL_RSA_GenAdd(WOLFSSL_RSA* rsa) int err; mp_int* t = NULL; #ifdef WOLFSSL_SMALL_STACK - mp_int *tmp = (mp_int *)XMALLOC(sizeof(*tmp), rsa->heap, - DYNAMIC_TYPE_TMP_BUFFER); - if (tmp == NULL) { - WOLFSSL_ERROR_MSG("Memory allocation failure"); - return -1; - } + mp_int *tmp = NULL; #else mp_int tmp[1]; #endif @@ -4475,6 +4470,17 @@ int wolfSSL_RSA_GenAdd(WOLFSSL_RSA* rsa) ret = -1; } +#ifdef WOLFSSL_SMALL_STACK + if (ret == 1) { + tmp = (mp_int *)XMALLOC(sizeof(*tmp), rsa->heap, + DYNAMIC_TYPE_TMP_BUFFER); + if (tmp == NULL) { + WOLFSSL_ERROR_MSG("Memory allocation failure"); + ret = -1; + } + } +#endif + if (ret == 1) { /* Initialize temp MP integer. */ if (mp_init(tmp) != MP_OKAY) { @@ -4523,7 +4529,8 @@ int wolfSSL_RSA_GenAdd(WOLFSSL_RSA* rsa) mp_clear(t); #ifdef WOLFSSL_SMALL_STACK - XFREE(tmp, rsa->heap, DYNAMIC_TYPE_TMP_BUFFER); + if (tmp != NULL) + XFREE(tmp, rsa->heap, DYNAMIC_TYPE_TMP_BUFFER); #endif return ret; diff --git a/src/ssl.c b/src/ssl.c index 2813d3b24..c026e7bb8 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -33285,7 +33285,7 @@ static int set_curves_list(WOLFSSL* ssl, WOLFSSL_CTX *ctx, const char* names) char name[MAX_CURVE_NAME_SZ]; byte groups_len = 0; #ifdef WOLFSSL_SMALL_STACK - void *heap = ssl? ssl->heap : ctx->heap; + void *heap = ssl? ssl->heap : ctx ? ctx->heap : NULL; int *groups; #else int groups[WOLFSSL_MAX_GROUP_COUNT]; diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 9875f5d32..dd124bb0b 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -23392,14 +23392,14 @@ int wc_PubKeyPemToDer(const unsigned char* pem, int pemSz, #ifdef WOLFSSL_CERT_GEN int wc_PemCertToDer_ex(const char* fileName, DerBuffer** der) { -#ifdef WOLFSSL_SMALL_STACK - byte staticBuffer[1]; /* force XMALLOC */ -#else +#ifndef WOLFSSL_SMALL_STACK byte staticBuffer[FILE_BUFFER_SIZE]; #endif - byte* fileBuf = staticBuffer; + byte* fileBuf; int ret = 0; - XFILE file = NULL; + XFILE file = XBADFILE; + int dynamic = 0; + long sz = 0; WOLFSSL_ENTER("wc_PemCertToDer"); @@ -23409,49 +23409,53 @@ int wc_PemCertToDer_ex(const char* fileName, DerBuffer** der) else { file = XFOPEN(fileName, "rb"); if (file == XBADFILE) { - ret = BUFFER_E; + ret = IO_FAILED_E; } } if (ret == 0) { - int dynamic = 0; - long sz = 0; - if (XFSEEK(file, 0, XSEEK_END) != 0) { - ret = BUFFER_E; + ret = IO_FAILED_E; } + } + if (ret == 0) { sz = XFTELL(file); + if (sz <= 0) { + ret = IO_FAILED_E; + } + } + if (ret == 0) { if (XFSEEK(file, 0, XSEEK_SET) != 0) { - ret = BUFFER_E; + ret = IO_FAILED_E; } - - if (ret < 0) { - /* intentionally left empty. */ - } - else if (sz <= 0) { - ret = BUFFER_E; - } - else if (sz > (long)sizeof(staticBuffer)) { + } + if (ret == 0) { +#ifndef WOLFSSL_SMALL_STACK + if (sz <= (long)sizeof(staticBuffer)) + fileBuf = staticBuffer; + else +#endif + { fileBuf = (byte*)XMALLOC(sz, NULL, DYNAMIC_TYPE_FILE); if (fileBuf == NULL) ret = MEMORY_E; else dynamic = 1; } - - if (ret == 0) { - if ((size_t)XFREAD(fileBuf, 1, sz, file) != (size_t)sz) { - ret = BUFFER_E; - } - else { - ret = PemToDer(fileBuf, sz, CA_TYPE, der, 0, NULL,NULL); - } - } - - XFCLOSE(file); - if (dynamic) - XFREE(fileBuf, NULL, DYNAMIC_TYPE_FILE); } + if (ret == 0) { + if ((size_t)XFREAD(fileBuf, 1, sz, file) != (size_t)sz) { + ret = IO_FAILED_E; + } + else { + ret = PemToDer(fileBuf, sz, CA_TYPE, der, 0, NULL,NULL); + } + } + + if (file != XBADFILE) + XFCLOSE(file); + if (dynamic) + XFREE(fileBuf, NULL, DYNAMIC_TYPE_FILE); return ret; } @@ -23479,16 +23483,14 @@ int wc_PemCertToDer(const char* fileName, unsigned char* derBuf, int derSz) /* load pem public key from file into der buffer, return der size or error */ int wc_PemPubKeyToDer_ex(const char* fileName, DerBuffer** der) { -#ifdef WOLFSSL_SMALL_STACK - byte staticBuffer[1]; /* force XMALLOC */ -#else +#ifndef WOLFSSL_SMALL_STACK byte staticBuffer[FILE_BUFFER_SIZE]; #endif - byte* fileBuf = staticBuffer; + byte* fileBuf; int dynamic = 0; int ret = 0; long sz = 0; - XFILE file = NULL; + XFILE file = XBADFILE; WOLFSSL_ENTER("wc_PemPubKeyToDer"); @@ -23498,26 +23500,33 @@ int wc_PemPubKeyToDer_ex(const char* fileName, DerBuffer** der) else { file = XFOPEN(fileName, "rb"); if (file == XBADFILE) { - ret = BUFFER_E; + ret = IO_FAILED_E; } } if (ret == 0) { if (XFSEEK(file, 0, XSEEK_END) != 0) { - ret = BUFFER_E; + ret = IO_FAILED_E; } } if (ret == 0) { sz = XFTELL(file); - if (XFSEEK(file, 0, XSEEK_SET) != 0) { - ret = BUFFER_E; + if (sz <= 0) { + ret = IO_FAILED_E; } } if (ret == 0) { - if (sz <= 0) { - ret = BUFFER_E; + if (XFSEEK(file, 0, XSEEK_SET) != 0) { + ret = IO_FAILED_E; } - else if (sz > (long)sizeof(staticBuffer)) { + } + if (ret == 0) { +#ifndef WOLFSSL_SMALL_STACK + if (sz <= (long)sizeof(staticBuffer)) + fileBuf = staticBuffer; + else +#endif + { fileBuf = (byte*)XMALLOC(sz, NULL, DYNAMIC_TYPE_FILE); if (fileBuf == NULL) ret = MEMORY_E; From 7c15131db5255a68343ea08bb7de3d4bb74683b3 Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Thu, 6 Apr 2023 15:15:26 -0500 Subject: [PATCH 2/2] wolfcrypt/src/asn.c: in wc_PemCertToDer_ex() and wc_PemPubKeyToDer_ex(), work around false positive -Wmaybe-uninitialized from scan-build. --- wolfcrypt/src/asn.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index dd124bb0b..ff841a10f 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -23395,7 +23395,7 @@ int wc_PemCertToDer_ex(const char* fileName, DerBuffer** der) #ifndef WOLFSSL_SMALL_STACK byte staticBuffer[FILE_BUFFER_SIZE]; #endif - byte* fileBuf; + byte* fileBuf = NULL; int ret = 0; XFILE file = XBADFILE; int dynamic = 0; @@ -23486,7 +23486,7 @@ int wc_PemPubKeyToDer_ex(const char* fileName, DerBuffer** der) #ifndef WOLFSSL_SMALL_STACK byte staticBuffer[FILE_BUFFER_SIZE]; #endif - byte* fileBuf; + byte* fileBuf = NULL; int dynamic = 0; int ret = 0; long sz = 0;