From 02cc7bf82eda119f3f33b1215a04642b810f7ee3 Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Sat, 17 Sep 2022 12:53:37 -0500 Subject: [PATCH 1/3] fix whitespace/linelength/indentation. --- src/dtls13.c | 2 +- src/tls13.c | 13 +++++++------ wolfcrypt/src/ext_kyber.c | 8 ++++---- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/dtls13.c b/src/dtls13.c index 0d27b1a40..526026f83 100644 --- a/src/dtls13.c +++ b/src/dtls13.c @@ -259,7 +259,7 @@ static int Dtls13GetRnMask(WOLFSSL* ssl, const byte* ciphertext, byte* mask, #ifdef HAVE_CHACHA if (ssl->specs.bulk_cipher_algorithm == wolfssl_chacha) { word32 counter; - int ret; + int ret; if (c->chacha == NULL) return BAD_STATE_E; diff --git a/src/tls13.c b/src/tls13.c index 271cdccd4..1f2625776 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -518,7 +518,7 @@ static int DeriveClientHandshakeSecret(WOLFSSL* ssl, byte* key) if (ssl == NULL || ssl->arrays == NULL) { return BAD_FUNC_ARG; } - + ret = Tls13DeriveKey(ssl, key, -1, ssl->arrays->preMasterSecret, clientHandshakeLabel, CLIENT_HANDSHAKE_LABEL_SZ, ssl->specs.mac_algorithm, 1); @@ -6169,9 +6169,10 @@ int SendTls13ServerHello(WOLFSSL* ssl, byte extMsgType) { #ifdef WOLFSSL_DTLS13 if (ssl->options.dtls) { - ret = Dtls13HashHandshake(ssl, - output + Dtls13GetRlHeaderLength(ssl, 0) , - (word16)sendSz - Dtls13GetRlHeaderLength(ssl, 0)); + ret = Dtls13HashHandshake( + ssl, + output + Dtls13GetRlHeaderLength(ssl, 0) , + (word16)sendSz - Dtls13GetRlHeaderLength(ssl, 0)); } else #endif /* WOLFSSL_DTLS13 */ @@ -6452,7 +6453,7 @@ static int SendTls13CertificateRequest(WOLFSSL* ssl, byte* reqCtx, ssl->options.buildingMsg = 0; ret = Dtls13HandshakeSend(ssl, output, (word16)sendSz, (word16)i, - certificate_request, 1); + certificate_request, 1); WOLFSSL_LEAVE("SendTls13CertificateRequest", ret); WOLFSSL_END(WC_FUNC_CERTIFICATE_REQUEST_SEND); @@ -7263,7 +7264,7 @@ static int SendTls13Certificate(WOLFSSL* ssl) ssl->options.buildingMsg = 0; ssl->fragOffset = 0; ret = Dtls13HandshakeSend(ssl, output, (word16)sendSz, (word16)i, - certificate, 1); + certificate, 1); } else #endif /* WOLFSSL_DTLS13 */ diff --git a/wolfcrypt/src/ext_kyber.c b/wolfcrypt/src/ext_kyber.c index 83435d291..ceedffdbe 100644 --- a/wolfcrypt/src/ext_kyber.c +++ b/wolfcrypt/src/ext_kyber.c @@ -240,7 +240,7 @@ int wc_KyberKey_CipherTextSize(KyberKey* key, word32* len) } #ifdef HAVE_LIBOQS - /* NOTE: SHAKE and AES variants have the same length ciphertext. */ + /* NOTE: SHAKE and AES variants have the same length ciphertext. */ if (ret == 0) { switch (key->type) { case KYBER_LEVEL1: @@ -534,7 +534,7 @@ int wc_KyberKey_Decapsulate(KyberKey* key, unsigned char* ss, /** * Decode the private key. - * + * * We store the whole thing in the private key buffer. Note this means we cannot * do the encapsulation operation with the private key. But generally speaking * this is never done. @@ -614,7 +614,7 @@ int wc_KyberKey_DecodePublicKey(KyberKey* key, unsigned char* in, word32 len) /** * Encode the private key. - * + * * We stored it as a blob so we can just copy it over. * * @param [in] key Kyber key object. @@ -664,7 +664,7 @@ int wc_KyberKey_EncodePrivateKey(KyberKey* key, unsigned char* out, word32 len) */ int wc_KyberKey_EncodePublicKey(KyberKey* key, unsigned char* out, word32 len) { - int ret = 0; + int ret = 0; unsigned int pubLen = 0; if ((key == NULL) || (out == NULL)) { From 0fc80f5f8502ac516137b0c536eb1738eb761608 Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Sat, 17 Sep 2022 12:55:48 -0500 Subject: [PATCH 2/3] wolfcrypt/src/sp_int.c: catch and propagate errors from sp_init_size() in sp_invmod() and sp_gcd() to fix clang-analyzer-core.UndefinedBinaryOperatorResult. --- wolfcrypt/src/sp_int.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/wolfcrypt/src/sp_int.c b/wolfcrypt/src/sp_int.c index 40ad95104..3f1a611ea 100644 --- a/wolfcrypt/src/sp_int.c +++ b/wolfcrypt/src/sp_int.c @@ -10600,11 +10600,16 @@ int sp_invmod(sp_int* a, sp_int* m, sp_int* r) else if (err != MP_OKAY) { } else { - sp_init_size(u, m->used + 1); - sp_init_size(v, m->used + 1); - sp_init_size(b, m->used + 1); - sp_init_size(c, 2 * m->used + 1); + err = sp_init_size(u, m->used + 1); + if (err == MP_OKAY) + err = sp_init_size(v, m->used + 1); + if (err == MP_OKAY) + err = sp_init_size(b, m->used + 1); + if (err == MP_OKAY) + err = sp_init_size(c, 2 * m->used + 1); + } + if ((err == MP_OKAY) && !sp_isone(a)) { if (sp_iseven(m)) { /* a^-1 mod m = m + ((1 - m*(m^-1 % a)) / a) */ mm = a; @@ -16363,10 +16368,14 @@ int sp_gcd(sp_int* a, sp_int* b, sp_int* r) u = d[0]; v = d[1]; t = d[2]; - sp_init_size(u, used); - sp_init_size(v, used); - sp_init_size(t, used); + err = sp_init_size(u, used); + } + if (err == MP_OKAY) + err = sp_init_size(v, used); + if (err == MP_OKAY) + err = sp_init_size(t, used); + if (err == MP_OKAY) { if (_sp_cmp(a, b) != MP_LT) { sp_copy(b, u); /* First iteration - u = a, v = b */ From ac0d7f4d84d3470f59f822f809714c41f79c8013 Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Sat, 17 Sep 2022 13:02:51 -0500 Subject: [PATCH 3/3] src/internal.c: in DtlsMsgNew(), iff WOLFSSL_ASYNC_CRYPT, allow sz==0 allocation, to fix infinite loop in ProcessReplyEx() around DoDtlsHandShakeMsg(); in DtlsMsgAssembleCompleteMessage() restore fix from 0603031362a for pointerOutOfBounds (undefined behavior) construct; in ProcessReplyEx(), in WOLFSSL_DTLS13 case ack, check and propagate error from DoDtls13Ack() (fix from @guidovranken). --- src/internal.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/internal.c b/src/internal.c index 8017a60f6..1384c7680 100644 --- a/src/internal.c +++ b/src/internal.c @@ -8152,10 +8152,12 @@ DtlsMsg* DtlsMsgNew(word32 sz, byte tx, void* heap) DtlsMsg* msg; WOLFSSL_ENTER("DtlsMsgNew()"); +#ifndef WOLFSSL_ASYNC_CRYPT if (sz == 0) { WOLFSSL_MSG("DtlsMsgNew: sz == 0 not allowed"); return NULL; } +#endif (void)heap; msg = (DtlsMsg*)XMALLOC(sizeof(DtlsMsg), heap, DYNAMIC_TYPE_DTLS_MSG); @@ -8386,8 +8388,18 @@ static void DtlsMsgAssembleCompleteMessage(DtlsMsg* msg) /* frag->padding makes sure we can fit the entire DTLS handshake header * before frag->buf */ - dtls = (DtlsHandShakeHeader*)(msg->fragBucketList->buf - - DTLS_HANDSHAKE_HEADER_SZ); + + /* note the dtls pointer needs to be computed from msg->fragBucketList, not + * from msg->fragBucketList->buf, to avoid a pointerOutOfBounds access + * detected by cppcheck. + * + * also note, the (void *) intermediate cast is necessary to avoid a + * potential -Wcast-align around alignment of DtlsHandShakeHeader exceeding + * alignment of char. + */ + dtls = (DtlsHandShakeHeader*)(void *)((char *)msg->fragBucketList + + OFFSETOF(DtlsFragBucket,buf) + - DTLS_HANDSHAKE_HEADER_SZ); msg->fragBucketList = NULL; msg->fragBucketListCount = 0; @@ -19839,6 +19851,8 @@ int ProcessReplyEx(WOLFSSL* ssl, int allowSocketErr) ssl->keys.padSz, &processedSize); ssl->buffers.inputBuffer.idx += processedSize; ssl->buffers.inputBuffer.idx += ssl->keys.padSz; + if (ret != 0) + return ret; break; } FALL_THROUGH; @@ -20694,7 +20708,7 @@ int BuildMessage(WOLFSSL* ssl, byte* output, int outSz, const byte* input, if (sizeOnly) goto exit_buildmsg; - { + { #if defined(HAVE_SECURE_RENEGOTIATION) && defined(WOLFSSL_DTLS) /* If we want the PREV_ORDER then modify CUR_ORDER sequence number * for all encryption algos that use it for encryption parameters */ @@ -20735,7 +20749,7 @@ int BuildMessage(WOLFSSL* ssl, byte* output, int outSz, const byte* input, ssl->keys.dtls_sequence_number_lo = dtls_sequence_number_lo; } #endif - } + } if (ret != 0) goto exit_buildmsg;