From 763ced668e4db6eb41540c5fee9d55e45ccccf8d Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Wed, 7 Aug 2024 13:13:43 -0500 Subject: [PATCH] fixes for defects identified by cppcheck and clang-tidy on --enable-debug builds: null deref in tests/api.c:load_pem_key_file_as_der(), redundant declarations in wolfcrypt/benchmark/benchmark.c, and numerous unchecked XSNPRINTF()s in wolfcrypt/src/logging.c and src/internal.c. --- src/internal.c | 6 +++- tests/api.c | 8 +++-- wolfcrypt/benchmark/benchmark.c | 12 ------- wolfcrypt/src/logging.c | 62 ++++++++++++++++++++++++++------- 4 files changed, 60 insertions(+), 28 deletions(-) diff --git a/src/internal.c b/src/internal.c index 16230dbb78..219465a1db 100644 --- a/src/internal.c +++ b/src/internal.c @@ -20653,7 +20653,11 @@ static void LogAlert(int type) typeStr = AlertTypeToString(type); if (typeStr != NULL) { char buff[60]; - XSNPRINTF(buff, sizeof(buff), "Alert type: %s", typeStr); + if (XSNPRINTF(buff, sizeof(buff), "Alert type: %s", typeStr) + >= (int)sizeof(buff)) + { + buff[sizeof(buff) - 1] = 0; + } WOLFSSL_MSG(buff); } #else diff --git a/tests/api.c b/tests/api.c index 04858591de..bb0877d100 100644 --- a/tests/api.c +++ b/tests/api.c @@ -78089,9 +78089,11 @@ static int load_pem_key_file_as_der(const char* privKeyFile, DerBuffer** pDer, (void)encInfo; /* not used in this test */ #ifdef DEBUG_WOLFSSL - fprintf(stderr, "%s (%d): Loading PEM %s (len %d) to DER (len %d)\n", - (ret == 0) ? "Success" : "Failure", ret, privKeyFile, (int)key_sz, - (*pDer)->length); + if (*pDer != NULL) { + fprintf(stderr, "%s (%d): Loading PEM %s (len %d) to DER (len %d)\n", + (ret == 0) ? "Success" : "Failure", ret, privKeyFile, + (int)key_sz, (*pDer)->length); + } #endif return ret; diff --git a/wolfcrypt/benchmark/benchmark.c b/wolfcrypt/benchmark/benchmark.c index 0bb5322370..9078c0bf6c 100644 --- a/wolfcrypt/benchmark/benchmark.c +++ b/wolfcrypt/benchmark/benchmark.c @@ -1663,18 +1663,6 @@ static const char* bench_result_words3[][5] = { const char *desc_extra); #endif -#if defined(DEBUG_WOLFSSL) && !defined(HAVE_VALGRIND) && \ - !defined(HAVE_STACK_SIZE) -#ifdef __cplusplus - extern "C" { -#endif - WOLFSSL_API int wolfSSL_Debugging_ON(void); - WOLFSSL_API void wolfSSL_Debugging_OFF(void); -#ifdef __cplusplus - } /* extern "C" */ -#endif -#endif - #if !defined(WC_NO_RNG) && \ ((!defined(NO_RSA) && !defined(WOLFSSL_RSA_VERIFY_ONLY)) \ || !defined(NO_DH) || defined(WOLFSSL_KEY_GEN) || defined(HAVE_ECC) \ diff --git a/wolfcrypt/src/logging.c b/wolfcrypt/src/logging.c index 7d7571ed82..710b68bfdf 100644 --- a/wolfcrypt/src/logging.c +++ b/wolfcrypt/src/logging.c @@ -471,26 +471,48 @@ void WOLFSSL_BUFFER(const byte* buffer, word32 length) while (buflen > 0) { int bufidx = 0; - XSNPRINTF(&line[bufidx], sizeof(line)-bufidx, "\t"); + if (XSNPRINTF(&line[bufidx], sizeof(line)-bufidx, "\t") + >= (int)sizeof(line) - bufidx) + { + return; + } bufidx++; for (i = 0; i < LINE_LEN; i++) { if (i < buflen) { - XSNPRINTF(&line[bufidx], sizeof(line)-bufidx, "%02x ", buffer[i]); + if (XSNPRINTF(&line[bufidx], sizeof(line)-bufidx, "%02x ", + buffer[i]) >= (int)sizeof(line) - bufidx) + { + return; + } } else { - XSNPRINTF(&line[bufidx], sizeof(line)-bufidx, " "); + if (XSNPRINTF(&line[bufidx], sizeof(line)-bufidx, " ") + >= (int)sizeof(line) - bufidx) + { + return; + } } bufidx += 3; } - XSNPRINTF(&line[bufidx], sizeof(line)-bufidx, "| "); + if (XSNPRINTF(&line[bufidx], sizeof(line)-bufidx, "| ") + >= (int)sizeof(line) - bufidx) + { + return; + } bufidx++; for (i = 0; i < LINE_LEN; i++) { if (i < buflen) { - XSNPRINTF(&line[bufidx], sizeof(line)-bufidx, - "%c", 31 < buffer[i] && buffer[i] < 127 ? buffer[i] : '.'); + if (XSNPRINTF(&line[bufidx], sizeof(line)-bufidx, + "%c", 31 < buffer[i] && buffer[i] < 127 + ? buffer[i] + : '.') + >= (int)sizeof(line) - bufidx) + { + return; + } bufidx++; } } @@ -506,7 +528,11 @@ void WOLFSSL_ENTER(const char* msg) { if (loggingEnabled) { char buffer[WOLFSSL_MAX_ERROR_SZ]; - XSNPRINTF(buffer, sizeof(buffer), "wolfSSL Entering %s", msg); + if (XSNPRINTF(buffer, sizeof(buffer), "wolfSSL Entering %s", msg) + >= (int)sizeof(buffer)) + { + buffer[sizeof(buffer) - 1] = 0; + } wolfssl_log(ENTER_LOG, NULL, 0, buffer); } } @@ -516,7 +542,11 @@ void WOLFSSL_ENTER2(const char *file, int line, const char* msg) { if (loggingEnabled) { char buffer[WOLFSSL_MAX_ERROR_SZ]; - XSNPRINTF(buffer, sizeof(buffer), "wolfSSL Entering %s", msg); + if (XSNPRINTF(buffer, sizeof(buffer), "wolfSSL Entering %s", msg) + >= (int)sizeof(buffer)) + { + buffer[sizeof(buffer) - 1] = 0; + } wolfssl_log(ENTER_LOG, file, line, buffer); } } @@ -527,8 +557,12 @@ void WOLFSSL_LEAVE(const char* msg, int ret) { if (loggingEnabled) { char buffer[WOLFSSL_MAX_ERROR_SZ]; - XSNPRINTF(buffer, sizeof(buffer), "wolfSSL Leaving %s, return %d", - msg, ret); + if (XSNPRINTF(buffer, sizeof(buffer), "wolfSSL Leaving %s, return %d", + msg, ret) + >= (int)sizeof(buffer)) + { + buffer[sizeof(buffer) - 1] = 0; + } wolfssl_log(LEAVE_LOG, NULL, 0, buffer); } } @@ -538,8 +572,12 @@ void WOLFSSL_LEAVE2(const char *file, int line, const char* msg, int ret) { if (loggingEnabled) { char buffer[WOLFSSL_MAX_ERROR_SZ]; - XSNPRINTF(buffer, sizeof(buffer), "wolfSSL Leaving %s, return %d", - msg, ret); + if (XSNPRINTF(buffer, sizeof(buffer), "wolfSSL Leaving %s, return %d", + msg, ret) + >= (int)sizeof(buffer)) + { + buffer[sizeof(buffer) - 1] = 0; + } wolfssl_log(LEAVE_LOG, file, line, buffer); } }