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.
TokTok · Jan 17, 2022 · 2d38e5b · 2d38e5b
1 parent 4ce02c0
commit 2d38e5b
Show file tree

Hide file tree

Showing 8 changed files with 15 additions and 16 deletions.
diff --git 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
@@ -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
@@ -1 +1 @@
-fb46c678adbe48e846286d9cb45b560e26f51cb7eccb99378c57e66c6c49732b  /usr/local/bin/tox-bootstrapd
+1ab15196cb6cec903e92ffb608fde3681d7b9fe4c03e3532e40b9002ed042f01  /usr/local/bin/tox-bootstrapd
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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",
     ],
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		fb46c678adbe48e846286d9cb45b560e26f51cb7eccb99378c57e66c6c49732b /usr/local/bin/tox-bootstrapd
		1ab15196cb6cec903e92ffb608fde3681d7b9fe4c03e3532e40b9002ed042f01 /usr/local/bin/tox-bootstrapd