From 5fbcbb6c8336dccdb027322986bdc53edf7876fe Mon Sep 17 00:00:00 2001 From: iphydf Date: Mon, 17 Jan 2022 10:01:26 +0000 Subject: [PATCH] cleanup: Remove uses of `strcpy` and `sprintf`. Use of `strcpy` in these particular cases was safe, but it's hard to tell and also useless. `strcpy` would effectively need to do another `strlen` which we already did. Also removed sprintf, which was also safe in this case but it's easier to be "obviously safe", especially for static analysers. --- .github/workflows/sonar-scan.yml | 3 +-- other/DHT_bootstrap.c | 5 ++--- other/analysis/run-clang-tidy | 1 - other/bootstrap_daemon/docker/tox-bootstrapd.sha256 | 2 +- other/bootstrap_daemon/src/config.c | 10 ++++++---- other/bootstrap_daemon/src/tox-bootstrapd.c | 6 ++---- other/bootstrap_node_packets.c | 4 ++-- other/bootstrap_node_packets.h | 2 +- testing/BUILD.bazel | 1 + 9 files changed, 16 insertions(+), 18 deletions(-) diff --git a/.github/workflows/sonar-scan.yml b/.github/workflows/sonar-scan.yml index a845a11d..4b78b751 100644 --- a/.github/workflows/sonar-scan.yml +++ b/.github/workflows/sonar-scan.yml @@ -47,5 +47,4 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} - run: | - sonar-scanner --define sonar.host.url="${{ env.SONAR_SERVER_URL }}" --define sonar.cfamily.build-wrapper-output="${{ env.BUILD_WRAPPER_OUT_DIR }}" + run: 'sonar-scanner --define sonar.host.url="${{ env.SONAR_SERVER_URL }}" --define sonar.cfamily.build-wrapper-output="${{ env.BUILD_WRAPPER_OUT_DIR }}"' diff --git a/other/DHT_bootstrap.c b/other/DHT_bootstrap.c index 18fefcbc..069fc419 100644 --- a/other/DHT_bootstrap.c +++ b/other/DHT_bootstrap.c @@ -144,7 +144,7 @@ int main(int argc, char *argv[]) Mono_Time *mono_time = mono_time_new(); DHT *dht = new_dht(logger, mono_time, new_networking(logger, ip, PORT), true); Onion *onion = new_onion(logger, mono_time, dht); - Onion_Announce *onion_a = new_onion_announce(logger, mono_time, dht); + const Onion_Announce *onion_a = new_onion_announce(logger, mono_time, dht); #ifdef DHT_NODE_EXTRA_PACKETS bootstrap_set_callbacks(dht_get_net(dht), DHT_VERSION_NUMBER, DHT_MOTD, sizeof(DHT_MOTD)); @@ -159,7 +159,6 @@ int main(int argc, char *argv[]) manage_keys(dht); printf("Public key: "); - uint32_t i; #ifdef TCP_RELAY_ENABLED #define NUM_PORTS 3 @@ -181,7 +180,7 @@ int main(int argc, char *argv[]) exit(1); } - for (i = 0; i < 32; i++) { + for (uint32_t i = 0; i < 32; ++i) { const uint8_t *const self_public_key = dht_get_self_public_key(dht); printf("%02X", self_public_key[i]); fprintf(file, "%02X", self_public_key[i]); diff --git a/other/analysis/run-clang-tidy b/other/analysis/run-clang-tidy index a847320f..52235093 100755 --- a/other/analysis/run-clang-tidy +++ b/other/analysis/run-clang-tidy @@ -53,7 +53,6 @@ ERRORS="$ERRORS,-bugprone-posix-return" ERRORS="$ERRORS,-bugprone-signed-char-misuse" ERRORS="$ERRORS,-cert-err34-c" ERRORS="$ERRORS,-cert-str34-c" -ERRORS="$ERRORS,-clang-analyzer-security.insecureAPI.strcpy" ERRORS="$ERRORS,-hicpp-uppercase-literal-suffix" ERRORS="$ERRORS,-readability-suspicious-call-argument" ERRORS="$ERRORS,-readability-uppercase-literal-suffix" diff --git a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 index d3b4cdeb..f49cac52 100644 --- a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 +++ b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 @@ -1 +1 @@ -fb46c678adbe48e846286d9cb45b560e26f51cb7eccb99378c57e66c6c49732b /usr/local/bin/tox-bootstrapd +01ff907eae6d12ec2fb597bc0d7bf2549aadf40a8b6bc608f0e910feabb97eec /usr/local/bin/tox-bootstrapd diff --git a/other/bootstrap_daemon/src/config.c b/other/bootstrap_daemon/src/config.c index b25e442e..a927a558 100644 --- a/other/bootstrap_daemon/src/config.c +++ b/other/bootstrap_daemon/src/config.c @@ -168,8 +168,9 @@ int get_general_config(const char *cfg_file_path, char **pid_file_path, char **k tmp_pid_file = DEFAULT_PID_FILE_PATH; } - *pid_file_path = (char *)malloc(strlen(tmp_pid_file) + 1); - strcpy(*pid_file_path, tmp_pid_file); + const size_t pid_file_path_len = strlen(tmp_pid_file) + 1; + *pid_file_path = (char *)malloc(pid_file_path_len); + memcpy(*pid_file_path, tmp_pid_file, pid_file_path_len); // Get keys file location const char *tmp_keys_file; @@ -180,8 +181,9 @@ int get_general_config(const char *cfg_file_path, char **pid_file_path, char **k tmp_keys_file = DEFAULT_KEYS_FILE_PATH; } - *keys_file_path = (char *)malloc(strlen(tmp_keys_file) + 1); - strcpy(*keys_file_path, tmp_keys_file); + const size_t keys_file_path_len = strlen(tmp_keys_file) + 1; + *keys_file_path = (char *)malloc(strlen(tmp_keys_file)); + memcpy(*keys_file_path, tmp_keys_file, keys_file_path_len); // Get IPv6 option if (config_lookup_bool(&cfg, NAME_ENABLE_IPV6, enable_ipv6) == CONFIG_FALSE) { diff --git a/other/bootstrap_daemon/src/tox-bootstrapd.c b/other/bootstrap_daemon/src/tox-bootstrapd.c index 583826c3..9b868912 100644 --- a/other/bootstrap_daemon/src/tox-bootstrapd.c +++ b/other/bootstrap_daemon/src/tox-bootstrapd.c @@ -105,10 +105,8 @@ static void print_public_key(const uint8_t *public_key) char buffer[2 * CRYPTO_PUBLIC_KEY_SIZE + 1]; int index = 0; - size_t i; - - for (i = 0; i < CRYPTO_PUBLIC_KEY_SIZE; i++) { - index += sprintf(buffer + index, "%02X", public_key[i]); + for (size_t i = 0; i < CRYPTO_PUBLIC_KEY_SIZE; i++) { + index += snprintf(buffer + index, sizeof(buffer) - index, "%02X", public_key[i]); } log_write(LOG_LEVEL_INFO, "Public Key: %s\n", buffer); diff --git a/other/bootstrap_node_packets.c b/other/bootstrap_node_packets.c index a60dcad2..7dfa4569 100644 --- a/other/bootstrap_node_packets.c +++ b/other/bootstrap_node_packets.c @@ -27,7 +27,7 @@ static int handle_info_request(void *object, IP_Port source, const uint8_t *pack return 1; } - Networking_Core *nc = (Networking_Core *)object; + const Networking_Core *nc = (const Networking_Core *)object; uint8_t data[1 + sizeof(bootstrap_version) + MAX_MOTD_LENGTH]; data[0] = BOOTSTRAP_INFO_PACKET_ID; @@ -42,7 +42,7 @@ static int handle_info_request(void *object, IP_Port source, const uint8_t *pack return 1; } -int bootstrap_set_callbacks(Networking_Core *net, uint32_t version, uint8_t *motd, uint16_t motd_length) +int bootstrap_set_callbacks(Networking_Core *net, uint32_t version, const uint8_t *motd, uint16_t motd_length) { if (motd_length > MAX_MOTD_LENGTH) { return -1; diff --git a/other/bootstrap_node_packets.h b/other/bootstrap_node_packets.h index ebf69261..c5bda217 100644 --- a/other/bootstrap_node_packets.h +++ b/other/bootstrap_node_packets.h @@ -15,6 +15,6 @@ #define MAX_MOTD_LENGTH 256 /* I recommend you use a maximum of 96 bytes. The hard maximum is this though. */ -int bootstrap_set_callbacks(Networking_Core *net, uint32_t version, uint8_t *motd, uint16_t motd_length); +int bootstrap_set_callbacks(Networking_Core *net, uint32_t version, const uint8_t *motd, uint16_t motd_length); #endif // C_TOXCORE_OTHER_BOOTSTRAP_NODE_PACKETS_H diff --git a/testing/BUILD.bazel b/testing/BUILD.bazel index 8f8fce5f..94f0b10b 100644 --- a/testing/BUILD.bazel +++ b/testing/BUILD.bazel @@ -13,6 +13,7 @@ sh_test( size = "small", srcs = ["//hs-tokstyle/tools:check-cimple"], args = ["$(locations %s)" % f for f in CIMPLE_FILES] + [ + "-Wno-enum-names", "+RTS", "-N3", ],