From cebb4da307dfcfed01adb188753069120fdf20ae Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Tue, 25 Jul 2023 11:31:01 -0500 Subject: [PATCH] fixes and workarounds for cppcheck 2.11 with uninitvar checks reactivated, and legacyUninitvar suppressed globally (as before): src/internal.c:wolfSSL_DtlsUpdateWindow(): shiftTooManyBitsSigned and integerOverflowCond (true positive, fixed); src/ssl.c:wolfSSL_GetSessionFromCache(): autoVariables (true positive, intentional and now suppressed); wolfcrypt/src/asn.c: several uninitvars in EccSpecifiedECDomainDecode(), wc_EccPrivateKeyDecode(), DecodeSingleResponse(), and DecodeResponseData() (false positives due to bug in cppcheck short circuit eval analysis, mitigated by refactoring && expressions to nested-if constructs that are semantically identical); src/ssl.c:wolfSSL_GetSessionFromCache(): nullPointer (false positive due to bug in cppcheck value flow analysis, workarounded). --- src/internal.c | 2 +- src/ssl.c | 7 ++++++- wolfcrypt/src/asn.c | 36 ++++++++++++++++++++++++------------ 3 files changed, 31 insertions(+), 14 deletions(-) diff --git a/src/internal.c b/src/internal.c index afc08ea235..234eddd2f2 100644 --- a/src/internal.c +++ b/src/internal.c @@ -16823,7 +16823,7 @@ int wolfSSL_DtlsUpdateWindow(word16 cur_hi, word32 cur_lo, diff %= DTLS_WORD_BITS; if (idx < WOLFSSL_DTLS_WINDOW_WORDS) - window[idx] |= (1 << diff); + window[idx] |= (1U << diff); } else { _DtlsUpdateWindowGTSeq(diff + 1, window); diff --git a/src/ssl.c b/src/ssl.c index b89a5b888e..95860bac9c 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -13684,7 +13684,8 @@ int wolfSSL_GetSessionFromCache(WOLFSSL* ssl, WOLFSSL_SESSION* output) #endif if (output->ticketLenAlloc) XFREE(output->ticket, output->heap, DYNAMIC_TYPE_SESSION_TICK); - output->ticket = tmpTicket; + output->ticket = tmpTicket; /* cppcheck-suppress autoVariables + */ output->ticketLenAlloc = PREALLOC_SESSION_TICKET_LEN; output->ticketLen = 0; tmpBufSet = 1; @@ -13772,6 +13773,10 @@ int wolfSSL_GetSessionFromCache(WOLFSSL* ssl, WOLFSSL_SESSION* output) #endif /* HAVE_SESSION_TICKET && WOLFSSL_TLS13 */ } + /* mollify confused cppcheck nullPointer warning. */ + if (sess == NULL) + error = WOLFSSL_FAILURE; + if (error == WOLFSSL_SUCCESS) { #if defined(HAVE_SESSION_TICKET) && defined(WOLFSSL_TLS13) error = wolfSSL_DupSessionEx(sess, output, 1, diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index d75c71bf9b..07709d8872 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -32124,15 +32124,19 @@ static int EccSpecifiedECDomainDecode(const byte* input, word32 inSz, } #ifndef WOLFSSL_NO_ASN_STRICT /* Only version 2 and above can have a seed. */ - if ((ret == 0) && (dataASN[ECCSPECIFIEDASN_IDX_PARAM_SEED].tag != 0) && + if (ret == 0) { + if ((dataASN[ECCSPECIFIEDASN_IDX_PARAM_SEED].tag != 0) && (version < 2)) { - ret = ASN_PARSE_E; + ret = ASN_PARSE_E; + } } #endif /* Only version 2 and above can have a hash algorithm. */ - if ((ret == 0) && (dataASN[ECCSPECIFIEDASN_IDX_HASH_SEQ].tag != 0) && + if (ret == 0) { + if ((dataASN[ECCSPECIFIEDASN_IDX_HASH_SEQ].tag != 0) && (version < 2)) { - ret = ASN_PARSE_E; + ret = ASN_PARSE_E; + } } if ((ret == 0) && (dataASN[ECCSPECIFIEDASN_IDX_COFACTOR].tag != 0)) { /* Store optional co-factor. */ @@ -32447,8 +32451,10 @@ int wc_EccPrivateKeyDecode(const byte* input, word32* inOutIdx, ecc_key* key, inOutIdx, inSz); } /* Only version 1 supported. */ - if ((ret == 0) && (version != 1)) { - ret = ASN_PARSE_E; + if (ret == 0) { + if (version != 1) { + ret = ASN_PARSE_E; + } } /* Curve Parameters are optional. */ if ((ret == 0) && (dataASN[ECCKEYASN_IDX_PARAMS].tag != 0)) { @@ -34416,8 +34422,10 @@ static int DecodeSingleResponse(byte* source, word32* ioIndex, word32 size, ret = ASN_PARSE_E; } /* Validate the issuer key hash length is the size required. */ - if ((ret == 0) && (issuerKeyHashLen != ocspDigestSize)) { - ret = ASN_PARSE_E; + if (ret == 0) { + if (issuerKeyHashLen != ocspDigestSize) { + ret = ASN_PARSE_E; + } } if (ret == 0) { /* Store serial size. */ @@ -34804,12 +34812,16 @@ static int DecodeResponseData(byte* source, word32* ioIndex, 1, source, ioIndex, size); } /* Only support v1 == 0 */ - if ((ret == 0) && (version != 0)) { - ret = ASN_PARSE_E; + if (ret == 0) { + if (version != 0) { + ret = ASN_PARSE_E; + } } /* Ensure date is a minimal size. */ - if ((ret == 0) && (dateSz < MIN_DATE_SIZE)) { - ret = ASN_PARSE_E; + if (ret == 0) { + if (dateSz < MIN_DATE_SIZE) { + ret = ASN_PARSE_E; + } } if (ret == 0) { /* TODO: use byName/byKey fields. */