From fa65da7bb028cef084d1603a389a1db7d455f24d Mon Sep 17 00:00:00 2001
From: Daniel Pouzzner <douzzer@wolfssl.com>
Date: Thu, 17 Oct 2024 00:06:32 -0500
Subject: [PATCH 1/2] analyzer-driven cleanups of --enable-dtls13
--enable-dtls-mtu --enable-dtls-frag-ch:
Dtls13HashClientHello(): fix wc_HashType handling;
Dtls13SendFragment(): fix identicalConditionAfterEarlyExit;
GetDtlsRecordHeader(): fix error handling around GetDtls13RecordHeader() (incorrectLogicOperator);
test_wolfSSL_dtls_stateless_maxfrag(): fix a clang-analyzer-core.NullDereference,
test_dtls_frag_ch(): fix a clang-diagnostic-embedded-directive,
test_AEAD_limit_client(): fix an united-data defect found by valgrind.
---
src/dtls13.c | 20 ++++++++++----------
src/internal.c | 4 ++--
tests/api.c | 13 +++++++------
3 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/src/dtls13.c b/src/dtls13.c
index f091ed62ff..6f2f014895 100644
--- a/src/dtls13.c
+++ b/src/dtls13.c
@@ -495,22 +495,25 @@ int Dtls13HashClientHello(const WOLFSSL* ssl, byte* hash, int* hashSz,
wc_HashAlg hashCtx;
int type = wolfSSL_GetHmacType_ex(specs);
+ if (type < 0)
+ return type;
+
header[0] = (byte)client_hello;
c32to24(length, header + 1);
- ret = wc_HashInit_ex(&hashCtx, type, ssl->heap, ssl->devId);
+ ret = wc_HashInit_ex(&hashCtx, (enum wc_HashType)type, ssl->heap, ssl->devId);
if (ret == 0) {
- ret = wc_HashUpdate(&hashCtx, type, header, OPAQUE32_LEN);
+ ret = wc_HashUpdate(&hashCtx, (enum wc_HashType)type, header, OPAQUE32_LEN);
if (ret == 0)
- ret = wc_HashUpdate(&hashCtx, type, body, length);
+ ret = wc_HashUpdate(&hashCtx, (enum wc_HashType)type, body, length);
if (ret == 0)
- ret = wc_HashFinal(&hashCtx, type, hash);
+ ret = wc_HashFinal(&hashCtx, (enum wc_HashType)type, hash);
if (ret == 0) {
- *hashSz = wc_HashGetDigestSize(type);
+ *hashSz = wc_HashGetDigestSize((enum wc_HashType)type);
if (*hashSz < 0)
ret = *hashSz;
}
- wc_HashFree(&hashCtx, type);
+ wc_HashFree(&hashCtx, (enum wc_HashType)type);
}
return ret;
}
@@ -568,9 +571,6 @@ static int Dtls13SendFragment(WOLFSSL* ssl, byte* output, word16 output_size,
else {
msg = output + recordHeaderLength;
- if (length <= recordHeaderLength)
- return BUFFER_ERROR;
-
if (hashOutput) {
ret = Dtls13HashHandshake(ssl, msg, recordLength);
if (ret != 0)
@@ -1713,7 +1713,7 @@ static int _Dtls13HandshakeRecv(WOLFSSL* ssl, byte* input, word32 size,
isFirst = fragOff == 0;
isComplete = isFirst && fragLength == messageLength;
- if (!isComplete && !Dtls13AcceptFragmented(ssl, handshakeType)) {
+ if (!isComplete && !Dtls13AcceptFragmented(ssl, (enum HandShakeType)handshakeType)) {
#ifdef WOLFSSL_DTLS_CH_FRAG
byte tls13 = 0;
/* check if the first CH fragment contains a valid cookie */
diff --git a/src/internal.c b/src/internal.c
index 38a79997a9..011dce6105 100644
--- a/src/internal.c
+++ b/src/internal.c
@@ -11471,8 +11471,8 @@ static int GetDtlsRecordHeader(WOLFSSL* ssl, word32* inOutIdx,
if (ssl->options.tls1_3) {
ret = GetDtls13RecordHeader(ssl, inOutIdx, rh, size);
if (ret == 0 ||
- ret != WC_NO_ERR_TRACE(SEQUENCE_ERROR) ||
- ret != WC_NO_ERR_TRACE(DTLS_CID_ERROR))
+ ((ret != WC_NO_ERR_TRACE(SEQUENCE_ERROR)) &&
+ (ret != WC_NO_ERR_TRACE(DTLS_CID_ERROR))))
return ret;
}
diff --git a/tests/api.c b/tests/api.c
index 8ce4bb97f1..cf2fa25fb7 100644
--- a/tests/api.c
+++ b/tests/api.c
@@ -87647,6 +87647,7 @@ static void test_AEAD_limit_client(WOLFSSL* ssl)
/* Test the sending limit for AEAD ciphers */
Dtls13GetEpoch(ssl, ssl->dtls13Epoch)->nextSeqNumber = sendLimit;
test_AEAD_seq_num = 1;
+ XMEMSET(msgBuf, 0, sizeof(msgBuf));
ret = wolfSSL_write(ssl, msgBuf, sizeof(msgBuf));
AssertIntGT(ret, 0);
didReKey = 0;
@@ -90812,14 +90813,13 @@ static int test_wolfSSL_dtls_stateless_maxfrag(void)
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
wolfDTLSv1_2_client_method, wolfDTLSv1_2_server_method), 0);
+ ExpectNotNull(ssl_s);
ExpectNotNull(ssl_c2 = wolfSSL_new(ctx_c));
ExpectIntEQ(wolfSSL_UseMaxFragment(ssl_c2, WOLFSSL_MFL_2_8),
WOLFSSL_SUCCESS);
wolfSSL_SetIOWriteCtx(ssl_c2, &test_ctx);
wolfSSL_SetIOReadCtx(ssl_c2, &test_ctx);
- if (ssl_s != NULL) {
- max_fragment = ssl_s->max_fragment;
- }
+ max_fragment = ssl_s->max_fragment;
/* send CH */
ExpectTrue((wolfSSL_connect(ssl_c2) == WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR)) &&
(ssl_c2->error == WC_NO_ERR_TRACE(WANT_READ)));
@@ -95173,11 +95173,12 @@ static int test_dtls_frag_ch(void)
/* Limit options to make the CH a fixed length */
/* See wolfSSL_parse_cipher_list for reason why we provide 1.3 AND 1.2
* ciphersuite. This is only necessary when building with OPENSSL_EXTRA. */
- ExpectTrue(wolfSSL_set_cipher_list(ssl_c, "TLS13-AES256-GCM-SHA384"
#ifdef OPENSSL_EXTRA
- ":DHE-RSA-AES256-GCM-SHA384"
+ ExpectTrue(wolfSSL_set_cipher_list(ssl_c, "TLS13-AES256-GCM-SHA384"
+ ":DHE-RSA-AES256-GCM-SHA384"));
+#else
+ ExpectTrue(wolfSSL_set_cipher_list(ssl_c, "TLS13-AES256-GCM-SHA384"));
#endif
- ));
/* CH1 */
ExpectIntEQ(wolfSSL_negotiate(ssl_c), -1);
From 06de22e72be26cef007b43b0bbc70fa2e1f1c08d Mon Sep 17 00:00:00 2001
From: Daniel Pouzzner <douzzer@wolfssl.com>
Date: Thu, 17 Oct 2024 10:57:19 -0500
Subject: [PATCH 2/2] api.c:test_wolfSSL_dtls_stateless_maxfrag(): add missing
condition (clang-analyzer-core.NullDereference).
---
tests/api.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tests/api.c b/tests/api.c
index cf2fa25fb7..6d765a8d52 100644
--- a/tests/api.c
+++ b/tests/api.c
@@ -90819,7 +90819,9 @@ static int test_wolfSSL_dtls_stateless_maxfrag(void)
WOLFSSL_SUCCESS);
wolfSSL_SetIOWriteCtx(ssl_c2, &test_ctx);
wolfSSL_SetIOReadCtx(ssl_c2, &test_ctx);
- max_fragment = ssl_s->max_fragment;
+ if (EXPECT_SUCCESS()) {
+ max_fragment = ssl_s->max_fragment;
+ }
/* send CH */
ExpectTrue((wolfSSL_connect(ssl_c2) == WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR)) &&
(ssl_c2->error == WC_NO_ERR_TRACE(WANT_READ)));