fix: Fix some uninitialised memory errors found by valgrind.

Also added a valgrind build to run it on every pull request. I've had to
disable a few tests because valgrind makes those run infinitely slowly,
consistently timing them out.

.cirrus.yml

@@ -92,6 +92,33 @@ bazel-tsan_task:
         -//c-toxcore/auto_tests:tcp_relay_test
         -//c-toxcore/auto_tests:tox_many_test
 
+# TODO(iphydf): Fix timeouts so we can run more of the tests disabled below.
+bazel-valgrind_task:
+  container:
+    image: toxchat/toktok-stack:0.0.31-release
+    cpu: 2
+    memory: 4G
+  configure_script:
+    - /src/workspace/tools/inject-repo c-toxcore
+  test_all_script:
+    - cd /src/workspace && bazel test -k
+        --remote_http_cache=http://$CIRRUS_HTTP_CACHE_HOST
+        --build_tag_filters=-haskell
+        --test_tag_filters=-haskell
+        --remote_download_minimal
+        --config=valgrind
+        --
+        //c-toxcore/...
+        -//c-toxcore/auto_tests:conference_av_test
+        -//c-toxcore/auto_tests:conference_test
+        -//c-toxcore/auto_tests:dht_test
+        -//c-toxcore/auto_tests:encryptsave_test
+        -//c-toxcore/auto_tests:file_transfer_test
+        -//c-toxcore/auto_tests:onion_test
+        -//c-toxcore/auto_tests:tcp_relay_test
+        -//c-toxcore/auto_tests:tox_many_tcp_test
+        -//c-toxcore/auto_tests:tox_many_test
+
 cimple_task:
   container:
     image: toxchat/toktok-stack:0.0.31-release

BUILD.bazel

@@ -8,29 +8,25 @@ project()
 genrule(
     name = "copy_headers",
     srcs = [
-        "//c-toxcore/toxav:public_headers",
-        "//c-toxcore/toxcore:public_headers",
-        "//c-toxcore/toxencryptsave:public_headers",
+        "//c-toxcore/toxav:toxav.h",
+        "//c-toxcore/toxcore:tox.h",
+        "//c-toxcore/toxencryptsave:toxencryptsave.h",
     ],
     outs = [
         "tox/toxav.h",
         "tox/tox.h",
         "tox/toxencryptsave.h",
     ],
     cmd = """
-        cp $(location //c-toxcore/toxav:public_headers) $(GENDIR)/c-toxcore/tox/toxav.h
-        cp $(location //c-toxcore/toxcore:public_headers) $(GENDIR)/c-toxcore/tox/tox.h
-        cp $(location //c-toxcore/toxencryptsave:public_headers) $(GENDIR)/c-toxcore/tox/toxencryptsave.h
+        cp $(location //c-toxcore/toxav:toxav.h) $(GENDIR)/c-toxcore/tox/toxav.h
+        cp $(location //c-toxcore/toxcore:tox.h) $(GENDIR)/c-toxcore/tox/tox.h
+        cp $(location //c-toxcore/toxencryptsave:toxencryptsave.h) $(GENDIR)/c-toxcore/tox/toxencryptsave.h
     """,
 )
 
 cc_library(
     name = "c-toxcore",
-    hdrs = [
-        "tox/tox.h",
-        "tox/toxav.h",
-        "tox/toxencryptsave.h",
-    ],
+    hdrs = [":copy_headers"],
     includes = ["."],
     visibility = ["//visibility:public"],
     deps = [

auto_tests/encryptsave_test.c

@@ -153,12 +153,16 @@ static void test_keys(void)
     int ciphertext_length2a = size_large + TOX_PASS_ENCRYPTION_EXTRA_LENGTH;
     int plaintext_length2a = size_large;
     uint8_t *encrypted2a = (uint8_t *)malloc(ciphertext_length2a);
+    ck_assert(encrypted2a != nullptr);
     uint8_t *in_plaintext2a = (uint8_t *)malloc(plaintext_length2a);
+    ck_assert(in_plaintext2a != nullptr);
+    random_bytes(in_plaintext2a, plaintext_length2a);
     ret = tox_pass_encrypt(in_plaintext2a, plaintext_length2a, key_char, 12, encrypted2a, &encerr);
     ck_assert_msg(ret, "tox_pass_encrypt failure 2a: %d", encerr);
 
     // Decryption of same message.
-    uint8_t *out_plaintext2a = (uint8_t *) malloc(plaintext_length2a);
+    uint8_t *out_plaintext2a = (uint8_t *)malloc(plaintext_length2a);
+    ck_assert(out_plaintext2a != nullptr);
     ret = tox_pass_decrypt(encrypted2a, ciphertext_length2a, key_char, 12, out_plaintext2a, &decerr);
     ck_assert_msg(ret, "tox_pass_decrypt failure 2a: %d", decerr);
     ck_assert_msg(memcmp(in_plaintext2a, out_plaintext2a, plaintext_length2a) == 0, "Large message decryption failed");

toxav/BUILD.bazel

@@ -3,15 +3,16 @@ load("//tools:no_undefined.bzl", "cc_library")
 
 package(features = ["layering_check"])
 
-filegroup(
-    name = "public_headers",
+exports_files(
     srcs = ["toxav.h"],
     visibility = ["//c-toxcore:__pkg__"],
 )
 
+# Private library with the public API header in it because in toxav, lots of
+# things depend on the public API header.
 cc_library(
-    name = "public",
-    hdrs = [":public_headers"],
+    name = "public_api",
+    hdrs = ["toxav.h"],
 )
 
 cc_library(
@@ -86,7 +87,7 @@ cc_library(
     srcs = ["audio.c"],
     hdrs = ["audio.h"],
     deps = [
-        ":public",
+        ":public_api",
         ":rtp",
         "//c-toxcore/toxcore:logger",
         "//c-toxcore/toxcore:mono_time",
@@ -107,7 +108,7 @@ cc_library(
     ],
     deps = [
         ":audio",
-        ":public",
+        ":public_api",
         ":ring_buffer",
         ":rtp",
         "//c-toxcore/toxcore:logger",

toxcore/BUILD.bazel

@@ -3,8 +3,7 @@ load("//tools:no_undefined.bzl", "cc_library")
 
 package(features = ["layering_check"])
 
-filegroup(
-    name = "public_headers",
+exports_files(
     srcs = ["tox.h"],
     visibility = ["//c-toxcore:__pkg__"],
 )

toxcore/DHT.c

@@ -513,6 +513,10 @@ int unpack_ip_port(IP_Port *ip_port, const uint8_t *data, uint16_t length, bool
         return -1;
     }
 
+    *ip_port = (IP_Port) {
+        0
+    };
+
     if (is_ipv4) {
         const uint32_t size = 1 + SIZE_IP4 + sizeof(uint16_t);
 

toxcore/crypto_core.c

@@ -68,7 +68,7 @@ static void crypto_free(uint8_t *ptr, size_t bytes)
 
     free(ptr);
 }
-#endif // !defined(FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION)
+#endif  // !defined(FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION)
 
 int32_t public_key_cmp(const uint8_t *pk1, const uint8_t *pk2)
 {
@@ -145,8 +145,10 @@ int32_t encrypt_data_symmetric(const uint8_t *secret_key, const uint8_t *nonce,
     }
 
 #ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
-    memcpy(encrypted, plain, length); // Don't encrypt anything
-    memset(encrypted + length, 0, crypto_box_MACBYTES); // Zero MAC to avoid false alarms of uninitialized memory
+    // Don't encrypt anything
+    memcpy(encrypted, plain, length);
+    // Zero MAC to avoid false alarms of uninitialized memory
+    memset(encrypted + length, 0, crypto_box_MACBYTES);
 #else
 
     const size_t size_temp_plain = length + crypto_box_ZEROBYTES;
@@ -189,7 +191,7 @@ int32_t decrypt_data_symmetric(const uint8_t *secret_key, const uint8_t *nonce,
     }
 
 #ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
-    memcpy(plain, encrypted, length - crypto_box_MACBYTES); // Don't encrypt anything
+    memcpy(plain, encrypted, length - crypto_box_MACBYTES);  // Don't encrypt anything
 #else
 
     const size_t size_temp_plain = length + crypto_box_ZEROBYTES;
@@ -320,7 +322,7 @@ int32_t crypto_new_keypair(uint8_t *public_key, uint8_t *secret_key)
 {
 #ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
     random_bytes(secret_key, CRYPTO_SECRET_KEY_SIZE);
-    memset(public_key, 0, CRYPTO_PUBLIC_KEY_SIZE); // Make MSAN happy
+    memset(public_key, 0, CRYPTO_PUBLIC_KEY_SIZE);  // Make MSAN happy
     crypto_scalarmult_curve25519_base(public_key, secret_key);
     return 0;
 #else

toxcore/network.h

@@ -138,12 +138,20 @@ typedef enum Net_Packet_Type {
 #define TCP_INET6 (TOX_AF_INET6 + 3)
 #define TCP_FAMILY (TOX_AF_INET6 + 4)
 
+#define SIZE_IP4 4
+#define SIZE_IP6 16
+#define SIZE_IP (1 + SIZE_IP6)
+#define SIZE_PORT 2
+#define SIZE_IPPORT (SIZE_IP + SIZE_PORT)
+
 typedef union IP4 {
     uint32_t uint32;
     uint16_t uint16[2];
     uint8_t uint8[4];
 } IP4;
 
+static_assert(sizeof(IP4) == SIZE_IP4, "IP4 size must be 4");
+
 IP4 get_ip4_loopback(void);
 extern const IP4 ip4_broadcast;
 
@@ -154,6 +162,10 @@ typedef union IP6 {
     uint64_t uint64[2];
 } IP6;
 
+// TODO(iphydf): Stop relying on this. We memcpy this struct (and IP4 above)
+// into packets but really should be serialising it properly.
+static_assert(sizeof(IP6) == SIZE_IP6, "IP6 size must be 16");
+
 IP6 get_ip6_loopback(void);
 extern const IP6 ip6_broadcast;
 
@@ -190,12 +202,6 @@ size_t net_unpack_u64(const uint8_t *bytes, uint64_t *v);
 /** Does the IP6 struct a contain an IPv4 address in an IPv6 one? */
 bool ipv6_ipv4_in_v6(IP6 a);
 
-#define SIZE_IP4 4
-#define SIZE_IP6 16
-#define SIZE_IP (1 + SIZE_IP6)
-#define SIZE_PORT 2
-#define SIZE_IPPORT (SIZE_IP + SIZE_PORT)
-
 #define TOX_ENABLE_IPV6_DEFAULT true
 
 /* addr_resolve return values */

toxcore/onion_client.c

@@ -270,6 +270,9 @@ static uint16_t random_nodes_path_onion(const Onion_Client *onion_c, Node_format
         }
 
         if (num_nodes >= 2) {
+            nodes[0] = (Node_format) {
+                0
+            };
             nodes[0].ip_port.ip.family = net_family_tcp_family;
             nodes[0].ip_port.ip.ip.v4.uint32 = random_tcp;
 
@@ -283,6 +286,9 @@ static uint16_t random_nodes_path_onion(const Onion_Client *onion_c, Node_format
                 return 0;
             }
 
+            nodes[0] = (Node_format) {
+                0
+            };
             nodes[0].ip_port.ip.family = net_family_tcp_family;
             nodes[0].ip_port.ip.ip.v4.uint32 = random_tcp;
 

toxencryptsave/BUILD.bazel

@@ -2,8 +2,7 @@ load("//tools:no_undefined.bzl", "cc_library")
 
 package(features = ["layering_check"])
 
-filegroup(
-    name = "public_headers",
+exports_files(
     srcs = ["toxencryptsave.h"],
     visibility = ["//c-toxcore:__pkg__"],
 )