From d930ecca4cf7bc76cc2ac1a50588a26d75431826 Mon Sep 17 00:00:00 2001 From: iphydf Date: Mon, 6 Dec 2021 18:41:27 +0000 Subject: [PATCH] chore: Run infer static analyser on circle ci builds. Also running some other analysis that we used to have on Travis. --- .circleci/config.yml | 50 +++++++++++++++++++++++++- .github/settings.yml | 5 +-- .travis/cmake-linux | 6 +--- other/analysis/gen-file.sh | 13 ++----- other/analysis/run-check-recursion | 7 ++++ other/analysis/run-clang | 1 + other/analysis/run-infer | 2 +- testing/DHT_test.c | 5 +-- toxcore/Messenger.c | 2 +- toxcore/ccompat.h | 3 ++ toxcore/crypto_core.c | 55 +++++++++++------------------ toxencryptsave/BUILD.bazel | 17 +++++++++ toxencryptsave/toxencryptsave.api.h | 4 +++ toxencryptsave/toxencryptsave.c | 54 ++++++++++++++-------------- toxencryptsave/toxencryptsave.h | 4 +++ 15 files changed, 142 insertions(+), 86 deletions(-) create mode 100755 other/analysis/run-check-recursion diff --git a/.circleci/config.yml b/.circleci/config.yml index 034fed8b..26551c2b 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -3,10 +3,14 @@ version: 2 workflows: version: 2 - clang-sanitizers: + program-analysis: jobs: + # Dynamic analysis - asan - tsan + # Static analysis + - static-analysis + - infer jobs: asan: @@ -41,3 +45,47 @@ jobs: - checkout - run: *apt_install - run: CC=clang .circleci/cmake-tsan + + infer: + working_directory: ~/work + docker: + - image: toxchat/infer + + steps: + - run: + apt-get update && + DEBIAN_FRONTEND=noninteractive + apt-get install -y --no-install-recommends + git + libopus-dev + libsodium-dev + libvpx-dev + pkg-config + - checkout + - run: infer --no-progress-bar -- cc toxav/*.c toxcore/*.c $(pkg-config --cflags opus vpx) + + static-analysis: + working_directory: ~/work + docker: + - image: ubuntu + + steps: + - checkout + - run: + apt-get update && + DEBIAN_FRONTEND=noninteractive + apt-get install -y --no-install-recommends + clang + cppcheck + g++ + libconfig-dev + libopus-dev + libsodium-dev + libvpx-dev + llvm + - run: other/analysis/check_logger_levels + - run: other/analysis/run-check-recursion + - run: other/analysis/run-clang + - run: other/analysis/run-clang-analyze + - run: other/analysis/run-cppcheck + - run: other/analysis/run-gcc diff --git a/.github/settings.yml b/.github/settings.yml index ec7e553e..b7828228 100644 --- a/.github/settings.yml +++ b/.github/settings.yml @@ -15,8 +15,9 @@ branches: - Codacy Static Code Analysis - CodeFactor - "ci/circleci: asan" - # TODO(iphydf): Find out why dht_test times out under tsan. - #- "ci/circleci: tsan" + - "ci/circleci: infer" + - "ci/circleci: static-analysis" + - "ci/circleci: tsan" - cimple - cirrus-ci - code-review/reviewable diff --git a/.travis/cmake-linux b/.travis/cmake-linux index 73b335fe..2c38792f 100755 --- a/.travis/cmake-linux +++ b/.travis/cmake-linux @@ -38,11 +38,7 @@ run_static_analysis() { export CPPFLAGS="-isystem $CACHEDIR/include" export LDFLAGS="-L$CACHEDIR/lib" - cat toxav/*.c toxcore/*.c toxencryptsave/*.c | - clang "$(pkg-config --cflags libsodium opus vpx)" \ - -Itoxav -Itoxcore -Itoxencryptsave -S -emit-llvm -xc - -o- | - opt -analyze -print-callgraph 2>&1 | - other/analysis/check_recursion + other/analysis/run-check_recursion other/analysis/run-clang other/analysis/run-clang-analyze } diff --git a/other/analysis/gen-file.sh b/other/analysis/gen-file.sh index 1c2a3f6d..29b9aa38 100644 --- a/other/analysis/gen-file.sh +++ b/other/analysis/gen-file.sh @@ -42,20 +42,11 @@ callmain() { : >amalgamation.cc -echo "#include " >>amalgamation.cc -echo "#include " >>amalgamation.cc -echo "#include " >>amalgamation.cc -echo "#include " >>amalgamation.cc - put auto_tests/check_compat.h -FIND_QUERY="find . '-(' -name '*.cc' -or -name '*.c' '-)'" +FIND_QUERY="find . '-(' -name '*.c' '-)'" FIND_QUERY="$FIND_QUERY -and -not -wholename './_build/*'" FIND_QUERY="$FIND_QUERY -and -not -wholename './super_donators/*'" -FIND_QUERY="$FIND_QUERY -and -not -wholename './toxav/*.cc'" -FIND_QUERY="$FIND_QUERY -and -not -wholename './toxcore/*.cc'" -FIND_QUERY="$FIND_QUERY -and -not -wholename './toxencryptsave/*.cc'" -FIND_QUERY="$FIND_QUERY -and -not -name amalgamation.cc" FIND_QUERY="$FIND_QUERY -and -not -name av_test.c" FIND_QUERY="$FIND_QUERY -and -not -name dht_test.c" FIND_QUERY="$FIND_QUERY -and -not -name version_test.c" @@ -64,7 +55,7 @@ readarray -t FILES <<<"$(eval "$FIND_QUERY")" (for i in "${FILES[@]}"; do grep -o '#include <[^>]*>' "$i" | - grep -E -v '|||>amalgamation.cc echo 'namespace {' >>amalgamation.cc diff --git a/other/analysis/run-check-recursion b/other/analysis/run-check-recursion new file mode 100755 index 00000000..741bac61 --- /dev/null +++ b/other/analysis/run-check-recursion @@ -0,0 +1,7 @@ +#!/bin/sh + +cat toxav/*.c toxcore/*.c toxencryptsave/*.c | + clang "$(pkg-config --cflags libsodium opus vpx)" \ + -Itoxav -Itoxcore -Itoxencryptsave -S -emit-llvm -xc - -o- | + opt -analyze -print-callgraph 2>&1 | + other/analysis/check_recursion diff --git a/other/analysis/run-clang b/other/analysis/run-clang index 59fc75a6..6f8e482e 100755 --- a/other/analysis/run-clang +++ b/other/analysis/run-clang @@ -9,6 +9,7 @@ clang++ -o /dev/null amalgamation.cc \ -std=c++11 \ -Werror \ -Weverything \ + -Wno-alloca \ -Wno-c++98-compat-pedantic \ -Wno-c99-extensions \ -Wno-cast-align \ diff --git a/other/analysis/run-infer b/other/analysis/run-infer index 3b168d08..0d9e33c6 100755 --- a/other/analysis/run-infer +++ b/other/analysis/run-infer @@ -5,4 +5,4 @@ SKIP_LINES=1 . other/analysis/gen-file.sh -infer -- clang++ -fsyntax-only amalgamation.cc "${CPPFLAGS[@]}" +infer --no-progress-bar -- clang++ -fsyntax-only amalgamation.cc "${CPPFLAGS[@]}" diff --git a/testing/DHT_test.c b/testing/DHT_test.c index 5f28eb99..f3a3076d 100644 --- a/testing/DHT_test.c +++ b/testing/DHT_test.c @@ -33,7 +33,7 @@ #define PORT 33445 -static uint8_t zeroes_cid[CRYPTO_PUBLIC_KEY_SIZE]; +static const uint8_t zeroes_cid[CRYPTO_PUBLIC_KEY_SIZE] = {0}; static void print_client_id(const uint8_t *public_key) { @@ -177,7 +177,8 @@ int main(int argc, char *argv[]) ip_init(&ip, ipv6enabled); Mono_Time *const mono_time = mono_time_new(); - DHT *dht = new_dht(nullptr, mono_time, new_networking(nullptr, ip, PORT), true); + Logger *const logger = logger_new(); + DHT *dht = new_dht(logger, mono_time, new_networking(logger, ip, PORT), true); printf("OUR ID: "); for (uint32_t i = 0; i < 32; i++) { diff --git a/toxcore/Messenger.c b/toxcore/Messenger.c index 982cecf7..22c3e9e8 100644 --- a/toxcore/Messenger.c +++ b/toxcore/Messenger.c @@ -910,7 +910,7 @@ static void check_friend_tcp_udp(Messenger *m, int32_t friendnumber, void *userd } } - m->friendlist[friendnumber].last_connection_udp_tcp = ret; + m->friendlist[friendnumber].last_connection_udp_tcp = (Connection_Status)ret; } static void break_files(const Messenger *m, int32_t friendnumber); diff --git a/toxcore/ccompat.h b/toxcore/ccompat.h index 8ff6563d..34def21b 100644 --- a/toxcore/ccompat.h +++ b/toxcore/ccompat.h @@ -42,6 +42,9 @@ #if !defined(__cplusplus) || __cplusplus < 201103L #define nullptr NULL +#ifndef static_assert +#define static_assert(cond, msg) extern int unused_for_static_assert +#endif #endif #ifdef __GNUC__ diff --git a/toxcore/crypto_core.c b/toxcore/crypto_core.c index 2fa4750f..35e3a603 100644 --- a/toxcore/crypto_core.c +++ b/toxcore/crypto_core.c @@ -33,41 +33,26 @@ #define crypto_box_MACBYTES (crypto_box_ZEROBYTES - crypto_box_BOXZEROBYTES) #endif -#if CRYPTO_PUBLIC_KEY_SIZE != crypto_box_PUBLICKEYBYTES -#error "CRYPTO_PUBLIC_KEY_SIZE should be equal to crypto_box_PUBLICKEYBYTES" -#endif - -#if CRYPTO_SECRET_KEY_SIZE != crypto_box_SECRETKEYBYTES -#error "CRYPTO_SECRET_KEY_SIZE should be equal to crypto_box_SECRETKEYBYTES" -#endif - -#if CRYPTO_SHARED_KEY_SIZE != crypto_box_BEFORENMBYTES -#error "CRYPTO_SHARED_KEY_SIZE should be equal to crypto_box_BEFORENMBYTES" -#endif - -#if CRYPTO_SYMMETRIC_KEY_SIZE != crypto_box_BEFORENMBYTES -#error "CRYPTO_SYMMETRIC_KEY_SIZE should be equal to crypto_box_BEFORENMBYTES" -#endif - -#if CRYPTO_MAC_SIZE != crypto_box_MACBYTES -#error "CRYPTO_MAC_SIZE should be equal to crypto_box_MACBYTES" -#endif - -#if CRYPTO_NONCE_SIZE != crypto_box_NONCEBYTES -#error "CRYPTO_NONCE_SIZE should be equal to crypto_box_NONCEBYTES" -#endif - -#if CRYPTO_SHA256_SIZE != crypto_hash_sha256_BYTES -#error "CRYPTO_SHA256_SIZE should be equal to crypto_hash_sha256_BYTES" -#endif - -#if CRYPTO_SHA512_SIZE != crypto_hash_sha512_BYTES -#error "CRYPTO_SHA512_SIZE should be equal to crypto_hash_sha512_BYTES" -#endif - -#if CRYPTO_PUBLIC_KEY_SIZE != 32 -#error "CRYPTO_PUBLIC_KEY_SIZE is required to be 32 bytes for public_key_cmp to work," -#endif +//!TOKSTYLE- +static_assert(CRYPTO_PUBLIC_KEY_SIZE == crypto_box_PUBLICKEYBYTES, + "CRYPTO_PUBLIC_KEY_SIZE should be equal to crypto_box_PUBLICKEYBYTES"); +static_assert(CRYPTO_SECRET_KEY_SIZE == crypto_box_SECRETKEYBYTES, + "CRYPTO_SECRET_KEY_SIZE should be equal to crypto_box_SECRETKEYBYTES"); +static_assert(CRYPTO_SHARED_KEY_SIZE == crypto_box_BEFORENMBYTES, + "CRYPTO_SHARED_KEY_SIZE should be equal to crypto_box_BEFORENMBYTES"); +static_assert(CRYPTO_SYMMETRIC_KEY_SIZE == crypto_box_BEFORENMBYTES, + "CRYPTO_SYMMETRIC_KEY_SIZE should be equal to crypto_box_BEFORENMBYTES"); +static_assert(CRYPTO_MAC_SIZE == crypto_box_MACBYTES, + "CRYPTO_MAC_SIZE should be equal to crypto_box_MACBYTES"); +static_assert(CRYPTO_NONCE_SIZE == crypto_box_NONCEBYTES, + "CRYPTO_NONCE_SIZE should be equal to crypto_box_NONCEBYTES"); +static_assert(CRYPTO_SHA256_SIZE == crypto_hash_sha256_BYTES, + "CRYPTO_SHA256_SIZE should be equal to crypto_hash_sha256_BYTES"); +static_assert(CRYPTO_SHA512_SIZE == crypto_hash_sha512_BYTES, + "CRYPTO_SHA512_SIZE should be equal to crypto_hash_sha512_BYTES"); +static_assert(CRYPTO_PUBLIC_KEY_SIZE == 32, + "CRYPTO_PUBLIC_KEY_SIZE is required to be 32 bytes for public_key_cmp to work"); +//!TOKSTYLE+ static uint8_t *crypto_malloc(size_t bytes) { diff --git a/toxencryptsave/BUILD.bazel b/toxencryptsave/BUILD.bazel index b1f9da0e..ab82f76c 100644 --- a/toxencryptsave/BUILD.bazel +++ b/toxencryptsave/BUILD.bazel @@ -33,3 +33,20 @@ cc_library( visibility = ["//c-toxcore/other:__pkg__"], deps = ["//c-toxcore/toxcore:crypto_core"], ) + +CIMPLE_SRCS = glob( + [ + "*.c", + "*.h", + ], + exclude = ["*.api.h"], +) + +sh_test( + name = "cimple_test", + size = "small", + srcs = ["//hs-tokstyle/tools:check-cimple"], + args = ["$(location %s)" % f for f in CIMPLE_SRCS], + data = CIMPLE_SRCS, + tags = ["haskell"], +) diff --git a/toxencryptsave/toxencryptsave.api.h b/toxencryptsave/toxencryptsave.api.h index b1068c50..df5d1136 100644 --- a/toxencryptsave/toxencryptsave.api.h +++ b/toxencryptsave/toxencryptsave.api.h @@ -10,6 +10,8 @@ #ifndef C_TOXCORE_TOXENCRYPTSAVE_TOXENCRYPTSAVE_H #define C_TOXCORE_TOXENCRYPTSAVE_TOXENCRYPTSAVE_H +//!TOKSTYLE- + #ifdef __cplusplus extern "C" { #endif @@ -303,5 +305,7 @@ typedef TOX_ERR_ENCRYPTION Tox_Err_Encryption; typedef TOX_ERR_DECRYPTION Tox_Err_Decryption; typedef TOX_ERR_GET_SALT Tox_Err_Get_Salt; +//!TOKSTYLE+ + #endif // C_TOXCORE_TOXENCRYPTSAVE_TOXENCRYPTSAVE_H %} diff --git a/toxencryptsave/toxencryptsave.c b/toxencryptsave/toxencryptsave.c index 70aa3746..be4bcaf2 100644 --- a/toxencryptsave/toxencryptsave.c +++ b/toxencryptsave/toxencryptsave.c @@ -28,17 +28,15 @@ #include #include -#if TOX_PASS_SALT_LENGTH != crypto_pwhash_scryptsalsa208sha256_SALTBYTES -#error TOX_PASS_SALT_LENGTH is assumed to be equal to crypto_pwhash_scryptsalsa208sha256_SALTBYTES -#endif - -#if TOX_PASS_KEY_LENGTH != CRYPTO_SHARED_KEY_SIZE -#error TOX_PASS_KEY_LENGTH is assumed to be equal to CRYPTO_SHARED_KEY_SIZE -#endif - -#if TOX_PASS_ENCRYPTION_EXTRA_LENGTH != (crypto_box_MACBYTES + crypto_box_NONCEBYTES + crypto_pwhash_scryptsalsa208sha256_SALTBYTES + TOX_ENC_SAVE_MAGIC_LENGTH) -#error TOX_PASS_ENCRYPTION_EXTRA_LENGTH is assumed to be equal to (crypto_box_MACBYTES + crypto_box_NONCEBYTES + crypto_pwhash_scryptsalsa208sha256_SALTBYTES + TOX_ENC_SAVE_MAGIC_LENGTH) -#endif +//!TOKSTYLE- +static_assert(TOX_PASS_SALT_LENGTH == crypto_pwhash_scryptsalsa208sha256_SALTBYTES, + "TOX_PASS_SALT_LENGTH is assumed to be equal to crypto_pwhash_scryptsalsa208sha256_SALTBYTES"); +static_assert(TOX_PASS_KEY_LENGTH == CRYPTO_SHARED_KEY_SIZE, + "TOX_PASS_KEY_LENGTH is assumed to be equal to CRYPTO_SHARED_KEY_SIZE"); +static_assert(TOX_PASS_ENCRYPTION_EXTRA_LENGTH == (crypto_box_MACBYTES + crypto_box_NONCEBYTES + + crypto_pwhash_scryptsalsa208sha256_SALTBYTES + TOX_ENC_SAVE_MAGIC_LENGTH), + "TOX_PASS_ENCRYPTION_EXTRA_LENGTH is assumed to be equal to (crypto_box_MACBYTES + crypto_box_NONCEBYTES + crypto_pwhash_scryptsalsa208sha256_SALTBYTES + TOX_ENC_SAVE_MAGIC_LENGTH)"); +//!TOKSTYLE+ uint32_t tox_pass_salt_length(void) { @@ -109,7 +107,7 @@ Tox_Pass_Key *tox_pass_key_derive(const uint8_t *passphrase, size_t pplength, Tox_Err_Key_Derivation *error) { uint8_t salt[crypto_pwhash_scryptsalsa208sha256_SALTBYTES]; - random_bytes(salt, sizeof salt); + random_bytes(salt, sizeof(salt)); return tox_pass_key_derive_with_salt(passphrase, pplength, salt, error); } @@ -129,10 +127,10 @@ Tox_Pass_Key *tox_pass_key_derive_with_salt(const uint8_t *passphrase, size_t pp uint8_t key[CRYPTO_SHARED_KEY_SIZE]; - /* Derive a key from the password */ - /* http://doc.libsodium.org/key_derivation/README.html */ - /* note that, according to the documentation, a generic pwhash interface will be created - * once the pwhash competition (https://password-hashing.net/) is over */ + // Derive a key from the password + // http://doc.libsodium.org/key_derivation/README.html + // note that, according to the documentation, a generic pwhash interface will be created + // once the pwhash competition (https://password-hashing.net/) is over */ if (crypto_pwhash_scryptsalsa208sha256( key, sizeof(key), (char *)passkey, sizeof(passkey), salt, crypto_pwhash_scryptsalsa208sha256_OPSLIMIT_INTERACTIVE * 2, /* slightly stronger */ @@ -157,7 +155,8 @@ Tox_Pass_Key *tox_pass_key_derive_with_salt(const uint8_t *passphrase, size_t pp return out_key; } -/* Encrypt arbitrary with a key produced by tox_derive_key_*. The output +/** + * Encrypt arbitrary with a key produced by `tox_derive_key_*`. The output * array must be at least data_len + TOX_PASS_ENCRYPTION_EXTRA_LENGTH bytes long. * key must be TOX_PASS_KEY_LENGTH bytes. * If you already have a symmetric key from somewhere besides this module, simply @@ -173,13 +172,12 @@ bool tox_pass_key_encrypt(const Tox_Pass_Key *key, const uint8_t *data, size_t d return 0; } - /* the output data consists of, in order: - * salt, nonce, mac, enc_data - * where the mac is automatically prepended by the encrypt() - * the salt+nonce is called the prefix - * I'm not sure what else I'm supposed to do with the salt and nonce, since we - * need them to decrypt the data - */ + // the output data consists of, in order: + // salt, nonce, mac, enc_data + // where the mac is automatically prepended by the encrypt() + // the salt+nonce is called the prefix + // I'm not sure what else I'm supposed to do with the salt and nonce, since we + // need them to decrypt the data /* first add the magic number */ memcpy(out, TOX_ENC_SAVE_MAGIC_NUMBER, TOX_ENC_SAVE_MAGIC_LENGTH); @@ -214,13 +212,13 @@ bool tox_pass_key_encrypt(const Tox_Pass_Key *key, const uint8_t *data, size_t d bool tox_pass_encrypt(const uint8_t *data, size_t data_len, const uint8_t *passphrase, size_t pplength, uint8_t *out, Tox_Err_Encryption *error) { - Tox_Err_Key_Derivation _error; - Tox_Pass_Key *key = tox_pass_key_derive(passphrase, pplength, &_error); + Tox_Err_Key_Derivation err; + Tox_Pass_Key *key = tox_pass_key_derive(passphrase, pplength, &err); if (!key) { - if (_error == TOX_ERR_KEY_DERIVATION_NULL) { + if (err == TOX_ERR_KEY_DERIVATION_NULL) { SET_ERROR_PARAMETER(error, TOX_ERR_ENCRYPTION_NULL); - } else if (_error == TOX_ERR_KEY_DERIVATION_FAILED) { + } else if (err == TOX_ERR_KEY_DERIVATION_FAILED) { SET_ERROR_PARAMETER(error, TOX_ERR_ENCRYPTION_KEY_DERIVATION_FAILED); } diff --git a/toxencryptsave/toxencryptsave.h b/toxencryptsave/toxencryptsave.h index 34287a1a..4b58165e 100644 --- a/toxencryptsave/toxencryptsave.h +++ b/toxencryptsave/toxencryptsave.h @@ -9,6 +9,8 @@ #ifndef C_TOXCORE_TOXENCRYPTSAVE_TOXENCRYPTSAVE_H #define C_TOXCORE_TOXENCRYPTSAVE_TOXENCRYPTSAVE_H +//!TOKSTYLE- + #ifdef __cplusplus extern "C" { #endif @@ -365,4 +367,6 @@ typedef TOX_ERR_ENCRYPTION Tox_Err_Encryption; typedef TOX_ERR_DECRYPTION Tox_Err_Decryption; typedef TOX_ERR_GET_SALT Tox_Err_Get_Salt; +//!TOKSTYLE+ + #endif // C_TOXCORE_TOXENCRYPTSAVE_TOXENCRYPTSAVE_H