From 9868acfa17e87beb94d75e5e8aa2b957e2a72b69 Mon Sep 17 00:00:00 2001 From: Jon Shallow Date: Fri, 19 Jul 2024 19:01:22 +0100 Subject: [PATCH] Scan-build: Add in CI Fix reported issues. Signed-off-by: Jon Shallow --- .github/workflows/main.yml | 13 +++++++++++++ ccm.c | 3 +++ dtls.c | 13 +++++++++++-- dtls_debug.c | 1 - sha2/sha2.c | 8 ++++++++ tests/dtls-client.c | 1 - tests/unit-tests/test_ecc.c | 1 + 7 files changed, 36 insertions(+), 4 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 7ac04b43..63c5e72f 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -24,6 +24,18 @@ jobs: with: python-version: 3.x - uses: pre-commit/action@v3.0.1 + scan-build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: setup + run: | + sudo apt-get update && sudo apt-get install -y libcunit1-dev libtool libtool-bin exuberant-ctags clang-tools + ./autogen.sh + - name: check + run: | + $GITHUB_WORKSPACE/configure --enable-tests + scan-build --status-bugs make EXTRA_CFLAGS=-DSCAN_BUILD_IGNORE_DEADSTORES build-linux-autotools: runs-on: ubuntu-latest @@ -91,6 +103,7 @@ jobs: cd build_test cmake -DWARNING_TO_ERROR=ON -Dmake_tests=ON .. cmake --build . + build-macos-cmake: name: Build for macOS using CMake runs-on: macos-latest diff --git a/ccm.c b/ccm.c index ac809593..d20e51af 100644 --- a/ccm.c +++ b/ccm.c @@ -287,6 +287,9 @@ dtls_ccm_decrypt_message(rijndael_ctx *ctx, size_t M, size_t L, msg += lm; } + /* Check that SET_COUNTER() does not overflow in L shift */ + assert(L <= 8); + /* calculate S_0 */ SET_COUNTER(A, L, 0, counter_tmp); rijndael_encrypt(ctx, A, S); diff --git a/dtls.c b/dtls.c index d9f39905..12c72823 100644 --- a/dtls.c +++ b/dtls.c @@ -2446,7 +2446,6 @@ dtls_check_ecdsa_signature_elem(uint8 *data, size_t data_length, if (ret <= 0) return ret; data += ret; - data_length -= ret; return data - data_orig; } @@ -2483,8 +2482,10 @@ check_client_certificate_verify(dtls_context_t *ctx, if (ret < 0) { return ret; } +#ifndef SCAN_BUILD_IGNORE_DEADSTORES data += ret; data_length -= ret; +#endif /* SCAN_BUILD_IGNORE_DEADSTORES */ copy_hs_hash(peer, &hs_hash); @@ -3088,6 +3089,7 @@ dtls_send_certificate_verify_ecdh(dtls_context_t *ctx, dtls_peer_t *peer, dtls_hash_finalize(sha256hash, &hs_hash); + assert(key); /* sign the ephemeral and its parameters */ dtls_ecdsa_create_sig_hash(key->priv_key, DTLS_EC_KEY_SIZE, sha256hash, sizeof(sha256hash), @@ -3487,7 +3489,9 @@ check_server_certificate(dtls_context_t *ctx, memcpy(config->keyx.ecdsa.other_pub_y, data, sizeof(config->keyx.ecdsa.other_pub_y)); +#ifndef SCAN_BUILD_IGNORE_DEADSTORES data += sizeof(config->keyx.ecdsa.other_pub_y); +#endif /* SCAN_BUILD_IGNORE_DEADSTORES */ err = CALL(ctx, verify_ecdsa_key, &peer->session, config->keyx.ecdsa.other_pub_x, @@ -3571,8 +3575,10 @@ check_server_key_exchange_ecdsa(dtls_context_t *ctx, if (ret < 0) { return ret; } +#ifndef SCAN_BUILD_IGNORE_DEADSTORES data += ret; data_length -= ret; +#endif /* SCAN_BUILD_IGNORE_DEADSTORES */ ret = dtls_ecdsa_verify_sig(config->keyx.ecdsa.other_pub_x, config->keyx.ecdsa.other_pub_y, sizeof(config->keyx.ecdsa.other_pub_x), @@ -3729,7 +3735,7 @@ check_server_hellodone(dtls_context_t *ctx, { int res; #ifdef DTLS_ECC - const dtls_ecdsa_key_t *ecdsa_key; + const dtls_ecdsa_key_t *ecdsa_key = NULL; #endif /* DTLS_ECC */ dtls_handshake_parameters_t *handshake = peer->handshake_params; @@ -4785,6 +4791,9 @@ dtls_handle_message(dtls_context_t *ctx, dtls_info("received close_notify alert, peer has been invalidated\n"); else dtls_warn("received fatal alert, peer has been invalidated\n"); + /* handle alert has invalidated peer */ + peer = NULL; + /* no more valid records after fatal alerts */ return 0; } else { dtls_stop_retransmission(ctx, peer); diff --git a/dtls_debug.c b/dtls_debug.c index f8521eaf..08bd1134 100644 --- a/dtls_debug.c +++ b/dtls_debug.c @@ -191,7 +191,6 @@ dsrv_print_addr(const session_t *addr, char *buf, size_t len) { return 0; } p += err; - len -= err; return p - buf; #else /* ! HAVE_INET_NTOP */ diff --git a/sha2/sha2.c b/sha2/sha2.c index 8744a37c..30ea5489 100644 --- a/sha2/sha2.c +++ b/sha2/sha2.c @@ -575,8 +575,10 @@ void dtls_sha256_transform(dtls_sha256_ctx* context, const sha2_byte* data) { context->state[6] += g; context->state[7] += h; +#ifndef SCAN_BUILD_IGNORE_DEADSTORES /* Clean up */ a = b = c = d = e = f = g = h = T1 = T2 = 0; +#endif /* SCAN_BUILD_IGNORE_DEADSTORES */ } #endif /* SHA2_UNROLL_TRANSFORM */ @@ -608,8 +610,10 @@ void dtls_sha256_update(dtls_sha256_ctx* context, const sha2_byte *data, size_t /* The buffer is not yet full */ MEMCPY_BCOPY(&context->buffer[usedspace], data, len); context->bitcount += len << 3; +#ifndef SCAN_BUILD_IGNORE_DEADSTORES /* Clean up: */ usedspace = freespace = 0; +#endif /* SCAN_BUILD_IGNORE_DEADSTORES */ return; } } @@ -625,8 +629,10 @@ void dtls_sha256_update(dtls_sha256_ctx* context, const sha2_byte *data, size_t MEMCPY_BCOPY(context->buffer, data, len); context->bitcount += len << 3; } +#ifndef SCAN_BUILD_IGNORE_DEADSTORES /* Clean up: */ usedspace = freespace = 0; +#endif /* SCAN_BUILD_IGNORE_DEADSTORES */ } void dtls_sha256_final(uint8_t digest[DTLS_SHA256_DIGEST_LENGTH], dtls_sha256_ctx* context) { @@ -686,7 +692,9 @@ void dtls_sha256_final(uint8_t digest[DTLS_SHA256_DIGEST_LENGTH], dtls_sha256_ct /* Clean up state data: */ MEMSET_BZERO(context, sizeof(*context)); +#ifndef SCAN_BUILD_IGNORE_DEADSTORES usedspace = 0; +#endif /* SCAN_BUILD_IGNORE_DEADSTORES */ } char *dtls_sha256_end(dtls_sha256_ctx* context, char buffer[DTLS_SHA256_DIGEST_STRING_LENGTH]) { diff --git a/tests/dtls-client.c b/tests/dtls-client.c index 5d856b61..27c12ef3 100644 --- a/tests/dtls-client.c +++ b/tests/dtls-client.c @@ -210,7 +210,6 @@ try_send(struct dtls_context_t *ctx, session_t *dst, size_t len, char *buf) { res = dtls_write(ctx, dst, (uint8 *)buf, len); if (res >= 0) { memmove(buf, buf + res, len - res); - len -= res; } } diff --git a/tests/unit-tests/test_ecc.c b/tests/unit-tests/test_ecc.c index a2dc99fe..2df006ab 100644 --- a/tests/unit-tests/test_ecc.c +++ b/tests/unit-tests/test_ecc.c @@ -184,6 +184,7 @@ t_test_ecc_ecdsa0(void) { ecc_ec_mult(BasePointx, BasePointy, ecdsaTestSecret, pub_x, pub_y); ret = ecc_ecdsa_sign(ecdsaTestSecret, ecdsaTestMessage, ecdsaTestRand1, tempx, tempy); + CU_ASSERT(ret == 0); memset(tempy, 0, sizeof(tempy)); ret = ecc_ecdsa_validate(pub_x, pub_y, ecdsaTestMessage, tempx, tempy);