From 930e1ac4737906b9444e7f9c7578adfbc805a9aa Mon Sep 17 00:00:00 2001 From: Kareem Date: Wed, 17 Nov 2021 13:54:02 -0700 Subject: [PATCH 1/5] Check ssl->arrays in SendClientHello to avoid null dereference. Allow building with fallthrough defined. --- src/internal.c | 4 ++++ wolfssl/wolfcrypt/types.h | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/internal.c b/src/internal.c index aca6eff7a..16d112d81 100644 --- a/src/internal.c +++ b/src/internal.c @@ -22666,6 +22666,10 @@ exit_dpk: WOLFSSL_START(WC_FUNC_CLIENT_HELLO_SEND); WOLFSSL_ENTER("SendClientHello"); + if (ssl == NULL || ssl->arrays == NULL) { + return BAD_FUNC_ARG; + } + if (ssl->suites == NULL) { WOLFSSL_MSG("Bad suites pointer in SendClientHello"); return SUITES_ERROR; diff --git a/wolfssl/wolfcrypt/types.h b/wolfssl/wolfcrypt/types.h index 6b6b544fd..4bcf24a9d 100644 --- a/wolfssl/wolfcrypt/types.h +++ b/wolfssl/wolfcrypt/types.h @@ -315,7 +315,8 @@ decouple library dependencies with standard string, memory and so on. #if defined(WOLFSSL_LINUXKM) && defined(fallthrough) #define FALL_THROUGH fallthrough #else - #define FALL_THROUGH ; __attribute__ ((fallthrough)) + /* Use __ notation to avoid conflicts */ + #define FALL_THROUGH ; __attribute__ ((__fallthrough__)) #endif #endif #endif From 07726359724af21a3a1fadb2f0dcb155af37a569 Mon Sep 17 00:00:00 2001 From: Kareem Date: Wed, 17 Nov 2021 17:44:12 -0700 Subject: [PATCH 2/5] Rework FALL_THROUGH definition to use fallthrough if defined. --- wolfssl/wolfcrypt/types.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/wolfssl/wolfcrypt/types.h b/wolfssl/wolfcrypt/types.h index 4bcf24a9d..a7dc69924 100644 --- a/wolfssl/wolfcrypt/types.h +++ b/wolfssl/wolfcrypt/types.h @@ -311,13 +311,13 @@ decouple library dependencies with standard string, memory and so on. #ifndef FALL_THROUGH /* GCC 7 has new switch() fall-through detection */ #if defined(__GNUC__) - #if ((__GNUC__ > 7) || ((__GNUC__ == 7) && (__GNUC_MINOR__ >= 1))) - #if defined(WOLFSSL_LINUXKM) && defined(fallthrough) - #define FALL_THROUGH fallthrough - #else - /* Use __ notation to avoid conflicts */ - #define FALL_THROUGH ; __attribute__ ((__fallthrough__)) - #endif + #if defined(fallthrough) + #define FALL_THROUGH fallthrough + #elif ((__GNUC__ > 7) || ((__GNUC__ == 7) && (__GNUC_MINOR__ >= 1))) + #define FALL_THROUGH ; __attribute__ ((fallthrough)) + #elif defined(__clang__) && defined(__clang_major__) && + (__clang_major__ >= 4) + #define FALL_THROUGH ; __attribute__ ((fallthrough)) #endif #endif #endif /* FALL_THROUGH */ From 72d4dcce0fe54ea0a82eb307d051b06af4aad2cf Mon Sep 17 00:00:00 2001 From: Kareem Date: Fri, 19 Nov 2021 14:13:02 -0700 Subject: [PATCH 3/5] Fix updated FALL_THROUGH macro. Fix a couple of case statements and remove a trailing whitespace. --- src/tls13.c | 2 +- wolfcrypt/src/evp.c | 1 + wolfcrypt/src/pkcs7.c | 2 +- wolfssl/wolfcrypt/types.h | 2 +- 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/tls13.c b/src/tls13.c index 47510a6c1..75761f40e 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -9676,7 +9676,7 @@ int wolfSSL_CTX_get_max_early_data(WOLFSSL_CTX* ctx) * * ssl The SSL/TLS object. * returns BAD_FUNC_ARG when ssl is NULL, or not using TLS v1.3, - * SIDE_ERROR when not a server and + * SIDE_ERROR when not a server and * returns the maximum amount of early data to be set */ int wolfSSL_get_max_early_data(WOLFSSL* ssl) diff --git a/wolfcrypt/src/evp.c b/wolfcrypt/src/evp.c index a604c7792..fffc35340 100644 --- a/wolfcrypt/src/evp.c +++ b/wolfcrypt/src/evp.c @@ -1964,6 +1964,7 @@ int wolfSSL_EVP_PKEY_keygen(WOLFSSL_EVP_PKEY_CTX *ctx, pkey->ownEcc = 1; } } + break; #endif default: break; diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index ff4ce4327..95091d21d 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -11682,8 +11682,8 @@ WOLFSSL_API int wc_PKCS7_DecodeAuthEnvelopedData(PKCS7* pkcs7, byte* in, wc_PKCS7_ChangeState(pkcs7, WC_PKCS7_AUTHENV_ATRBEND); FALL_THROUGH; -authenv_atrbend: case WC_PKCS7_AUTHENV_ATRBEND: +authenv_atrbend: #ifndef NO_PKCS7_STREAM if ((ret = wc_PKCS7_AddDataToStream(pkcs7, in, inSz, MAX_LENGTH_SZ + ASN_TAG_SZ, &pkiMsg, &idx)) != 0) { diff --git a/wolfssl/wolfcrypt/types.h b/wolfssl/wolfcrypt/types.h index a7dc69924..abbf1c084 100644 --- a/wolfssl/wolfcrypt/types.h +++ b/wolfssl/wolfcrypt/types.h @@ -315,7 +315,7 @@ decouple library dependencies with standard string, memory and so on. #define FALL_THROUGH fallthrough #elif ((__GNUC__ > 7) || ((__GNUC__ == 7) && (__GNUC_MINOR__ >= 1))) #define FALL_THROUGH ; __attribute__ ((fallthrough)) - #elif defined(__clang__) && defined(__clang_major__) && + #elif defined(__clang__) && defined(__clang_major__) && \ (__clang_major__ >= 4) #define FALL_THROUGH ; __attribute__ ((fallthrough)) #endif From fd6d4798881dfb700fac615e722dd01eb567e7a4 Mon Sep 17 00:00:00 2001 From: Kareem Date: Fri, 19 Nov 2021 14:19:27 -0700 Subject: [PATCH 4/5] Rework ssl and ssl->arrays NULL checks, and add to SendTls13ClientHello as well. --- src/internal.c | 12 ++++++++---- src/tls13.c | 7 +++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/internal.c b/src/internal.c index 16d112d81..daacabe0d 100644 --- a/src/internal.c +++ b/src/internal.c @@ -22658,6 +22658,10 @@ exit_dpk: int ret; word16 extSz = 0; + if (ssl == NULL) { + return BAD_FUNC_ARG; + } + #ifdef WOLFSSL_TLS13 if (IsAtLeastTLSv1_3(ssl->version)) return SendTls13ClientHello(ssl); @@ -22666,10 +22670,6 @@ exit_dpk: WOLFSSL_START(WC_FUNC_CLIENT_HELLO_SEND); WOLFSSL_ENTER("SendClientHello"); - if (ssl == NULL || ssl->arrays == NULL) { - return BAD_FUNC_ARG; - } - if (ssl->suites == NULL) { WOLFSSL_MSG("Bad suites pointer in SendClientHello"); return SUITES_ERROR; @@ -22719,6 +22719,10 @@ exit_dpk: #endif sendSz = length + HANDSHAKE_HEADER_SZ + RECORD_HEADER_SZ; + if (ssl->arrays == NULL) { + return BAD_FUNC_ARG; + } + #ifdef WOLFSSL_DTLS if (ssl->options.dtls) { length += ENUM_LEN; /* cookie */ diff --git a/src/tls13.c b/src/tls13.c index 75761f40e..04428314f 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -3014,6 +3014,10 @@ int SendTls13ClientHello(WOLFSSL* ssl) WOLFSSL_START(WC_FUNC_CLIENT_HELLO_SEND); WOLFSSL_ENTER("SendTls13ClientHello"); + if (ssl == NULL) { + return BAD_FUNC_ARG; + } + #ifdef HAVE_SESSION_TICKET if (ssl->options.resuming && (ssl->session.version.major != ssl->version.major || @@ -3130,6 +3134,9 @@ int SendTls13ClientHello(WOLFSSL* ssl) /* Keep for downgrade. */ ssl->chVersion = ssl->version; + if (ssl->arrays == NULL) { + return BAD_FUNC_ARG; + } /* Client Random */ if (ssl->options.connectState == CONNECT_BEGIN) { ret = wc_RNG_GenerateBlock(ssl->rng, args->output + args->idx, RAN_LEN); From 8de281c1d4d9e7c8c74d669ee620269db4d03fe5 Mon Sep 17 00:00:00 2001 From: Kareem Date: Fri, 19 Nov 2021 15:16:56 -0700 Subject: [PATCH 5/5] Fix minimum clang version for FALL_THROUGH. Not working properly before clang 11. --- wolfssl/wolfcrypt/types.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wolfssl/wolfcrypt/types.h b/wolfssl/wolfcrypt/types.h index abbf1c084..0f346cff4 100644 --- a/wolfssl/wolfcrypt/types.h +++ b/wolfssl/wolfcrypt/types.h @@ -316,7 +316,7 @@ decouple library dependencies with standard string, memory and so on. #elif ((__GNUC__ > 7) || ((__GNUC__ == 7) && (__GNUC_MINOR__ >= 1))) #define FALL_THROUGH ; __attribute__ ((fallthrough)) #elif defined(__clang__) && defined(__clang_major__) && \ - (__clang_major__ >= 4) + (__clang_major__ >= 11) #define FALL_THROUGH ; __attribute__ ((fallthrough)) #endif #endif