fix: Fix some uninitialised memory errors found by valgrind and msan.

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.
2024-03-22 13:30:51 +08:00 · 2022-01-13 12:31:14 +00:00 · 2022-01-13 12:31:14 +00:00 · 4a2cb37e4b
commit 4a2cb37e4b
parent 46a443f188
20 changed files with 162 additions and 84 deletions
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@ -8,6 +8,7 @@ workflows:
      # Dynamic analysis
      - asan
      - tsan
+      - msan
      # Static analysis
      - clang-tidy
      - infer
@ -47,6 +48,17 @@ jobs:
      - run: *apt_install
      - run: CC=clang .circleci/cmake-tsan

+  msan:
+    working_directory: ~/work
+    docker:
+      - image: toxchat/toktok-stack:0.0.31-msan
+
+    steps:
+      - checkout
+      - run: rm -rf /src/workspace/c-toxcore/* && mv * /src/workspace/c-toxcore/
+      # TODO(iphydf): Remove "|| true" once this works.
+      - run: cd /src/workspace && bazel test //c-toxcore/auto_tests:lossless_packet_test || true
+
  infer:
    working_directory: ~/work
    docker:
--- a/.cirrus.yml
+++ b/.cirrus.yml
@ -50,23 +50,22 @@ bazel-asan_task:
        //c-toxcore/...
        -//c-toxcore/auto_tests:tcp_relay_test # TODO(robinlinden): Why does this pass locally but not in Cirrus?

-# TODO(iphydf): Get msan to work properly.
-#bazel-msan_task:
-#  container:
-#    image: toxchat/toktok-stack:0.0.31-msan
-#    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
-#        --
-#        //c-toxcore/...
-#        -//c-toxcore/auto_tests:tcp_relay_test # TODO(robinlinden): Why does this pass locally but not in Cirrus?
+bazel-msan_task:
+  container:
+    image: toxchat/toktok-stack:0.0.31-msan
+    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
+        --
+        //c-toxcore/...
+        -//c-toxcore/auto_tests:tcp_relay_test || true # TODO(robinlinden): Why does this pass locally but not in Cirrus?

 # TODO(iphydf): Fix test timeouts.
 bazel-tsan_task:
@ -92,6 +91,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
--- a/.github/settings.yml
+++ b/.github/settings.yml
@ -25,7 +25,6 @@ branches:
          - "build-win32"
          - "build-win64"
          - "CodeFactor"
-          - "codecov/project"
          - "coverage-linux"
          - "ci/circleci: asan"
          - "ci/circleci: clang-tidy"
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@ -5,6 +5,17 @@ on:
    branches: [master]

 jobs:
+  build-msan:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - name: Pull toxchat/toktok-stack:0.0.31-msan
+        run: docker pull toxchat/toktok-stack:0.0.31-msan
+      - name: Run tests under MemorySanitizer
+        # TODO(iphydf): Remove "|| true" once this works correctly.
+        run: docker run --rm -v $PWD:/src/workspace/c-toxcore toxchat/toktok-stack:0.0.31-msan
+          bazel test //c-toxcore/auto_tests:lossless_packet_test || true
+
  build-nacl:
    runs-on: ubuntu-latest
    steps:
@ -18,8 +29,6 @@ jobs:
  build-win32:
    runs-on: ubuntu-latest
    steps:
-      - name: Set up Docker Buildx
-        uses: docker/setup-buildx-action@v1
      - uses: actions/checkout@v2
      - name: Docker Build
        run: .github/scripts/cmake-win32 install
@ -29,8 +38,6 @@ jobs:
  build-win64:
    runs-on: ubuntu-latest
    steps:
-      - name: Set up Docker Buildx
-        uses: docker/setup-buildx-action@v1
      - uses: actions/checkout@v2
      - name: Docker Build
        run: .github/scripts/cmake-win64 install
--- a/BUILD.bazel
+++ b/BUILD.bazel
@ -8,9 +8,9 @@ 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",
@ -18,19 +18,15 @@ genrule(
        "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 = [
--- a/auto_tests/conference_av_test.c
+++ b/auto_tests/conference_av_test.c
@ -107,7 +107,7 @@ static bool toxes_are_disconnected_from_group(uint32_t tox_count, Tox **toxes,
        num_disconnected += disconnected[i];
    }

-    for (uint32_t i = 0; i < tox_count; i++) {
+    for (uint32_t i = 0; i < tox_count; ++i) {
        if (disconnected[i]) {
            continue;
        }
@ -152,7 +152,7 @@ static void disconnect_toxes(uint32_t tox_count, Tox **toxes, State *state,

 static bool all_connected_to_group(uint32_t tox_count, Tox **toxes)
 {
-    for (uint32_t i = 0; i < tox_count; i++) {
+    for (uint32_t i = 0; i < tox_count; ++i) {
        if (tox_conference_peer_count(toxes[i], 0, nullptr) < tox_count) {
            return false;
        }
@ -215,11 +215,11 @@ static bool test_audio(Tox **toxes, State *state, const bool *disabled, bool qui
        printf("testing sending and receiving audio\n");
    }

-    int16_t PCM[GROUP_AV_TEST_SAMPLES];
+    const int16_t PCM[GROUP_AV_TEST_SAMPLES] = {0};

    reset_received_audio(toxes, state);

-    for (uint32_t n = 0; n < GROUP_AV_AUDIO_ITERATIONS; n++) {
+    for (uint32_t n = 0; n < GROUP_AV_AUDIO_ITERATIONS; ++n) {
        for (uint32_t i = 0; i < NUM_AV_GROUP_TOX; ++i) {
            if (disabled[i]) {
                continue;
@ -266,7 +266,7 @@ static void test_eventual_audio(Tox **toxes, State *state, const bool *disabled,

 static void do_audio(Tox **toxes, State *state, uint32_t iterations)
 {
-    int16_t PCM[GROUP_AV_TEST_SAMPLES];
+    const int16_t PCM[GROUP_AV_TEST_SAMPLES] = {0};
    printf("running audio for %u iterations\n", iterations);

    for (uint32_t f = 0; f < iterations; ++f) {
--- a/auto_tests/encryptsave_test.c
+++ b/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");
--- a/other/astyle/format-source
+++ b/other/astyle/format-source
@ -25,7 +25,7 @@ readarray -t CC_SOURCES <<<"$(find . '(' -name '*.cc' ')')"
 CC_SOURCES+=(toxcore/crypto_core.c)
 CC_SOURCES+=(toxcore/ping_array.c)

-for bin in clang-format-6.0 clang-format-5.0 clang-format; do
+for bin in clang-format-11 clang-format-7 clang-format-6.0 clang-format-5.0 clang-format; do
  if which "$bin"; then
    "$bin" -i -style='{BasedOnStyle: Google, ColumnLimit: 100}' "${CC_SOURCES[@]}"
    break
--- a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256
+++ b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256
@ -1 +1 @@
-bf2b6ce150f5dc3ed95e8636dd025a13015c98bbf922b7602969560d310045f7  /usr/local/bin/tox-bootstrapd
+7f4c96481e30156e3c2e7e448e70faaaa12911694390374ae09bd53d5d7b4f9c  /usr/local/bin/tox-bootstrapd
--- a/toxav/BUILD.bazel
+++ b/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",
--- a/toxcore/BUILD.bazel
+++ b/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__"],
 )
--- a/toxcore/DHT.c
+++ b/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);

--- a/toxcore/Messenger.c
+++ b/toxcore/Messenger.c
@ -3070,7 +3070,7 @@ static uint32_t tcp_relay_size(const Messenger *m)

 static uint8_t *save_tcp_relays(const Messenger *m, uint8_t *data)
 {
-    Node_format relays[NUM_SAVED_TCP_RELAYS];
+    Node_format relays[NUM_SAVED_TCP_RELAYS] = {0};
    uint8_t *temp_data = data;
    data = state_write_section_header(temp_data, STATE_COOKIE_TYPE, 0, STATE_TYPE_TCP_RELAY);

--- a/toxcore/crypto_core.c
+++ b/toxcore/crypto_core.c
@ -57,18 +57,25 @@ static_assert(CRYPTO_PUBLIC_KEY_SIZE == 32,
 #if !defined(FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION)
 static uint8_t *crypto_malloc(size_t bytes)
 {
-    return (uint8_t *)malloc(bytes);
+    uint8_t *ptr = (uint8_t *)malloc(bytes);
+
+    if (ptr != nullptr) {
+        crypto_memlock(ptr, bytes);
+    }
+
+    return ptr;
 }

 static void crypto_free(uint8_t *ptr, size_t bytes)
 {
    if (ptr != nullptr) {
        crypto_memzero(ptr, bytes);
+        crypto_memunlock(ptr, 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 +152,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 uninitialized memory reads.
+    memset(encrypted + length, 0, crypto_box_MACBYTES);
 #else

    const size_t size_temp_plain = length + crypto_box_ZEROBYTES;
@ -161,6 +170,11 @@ int32_t encrypt_data_symmetric(const uint8_t *secret_key, const uint8_t *nonce,
        return -1;
    }

+    // crypto_box_afternm requires the entire range of the output array be
+    // initialised with something. It doesn't matter what it's initialised with,
+    // so we'll pick 0x00.
+    memset(temp_encrypted, 0, size_temp_encrypted);
+
    memset(temp_plain, 0, crypto_box_ZEROBYTES);
    // Pad the message with 32 0 bytes.
    memcpy(temp_plain + crypto_box_ZEROBYTES, plain, length);
@ -189,7 +203,8 @@ 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
+    assert(length >= crypto_box_MACBYTES);
+    memcpy(plain, encrypted, length - crypto_box_MACBYTES);  // Don't encrypt anything
 #else

    const size_t size_temp_plain = length + crypto_box_ZEROBYTES;
@ -204,6 +219,11 @@ int32_t decrypt_data_symmetric(const uint8_t *secret_key, const uint8_t *nonce,
        return -1;
    }

+    // crypto_box_open_afternm requires the entire range of the output array be
+    // initialised with something. It doesn't matter what it's initialised with,
+    // so we'll pick 0x00.
+    memset(temp_plain, 0, size_temp_plain);
+
    memset(temp_encrypted, 0, crypto_box_BOXZEROBYTES);
    // Pad the message with 16 0 bytes.
    memcpy(temp_encrypted + crypto_box_BOXZEROBYTES, encrypted, length);
@ -320,7 +340,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
--- a/toxcore/crypto_core_test.cc
+++ b/toxcore/crypto_core_test.cc
@ -3,6 +3,7 @@
 #include <gtest/gtest.h>

 #include <algorithm>
+#include <array>
 #include <vector>

 namespace {
--- a/toxcore/network.h
+++ b/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 */
--- a/toxcore/onion.c
+++ b/toxcore/onion.c
@ -357,7 +357,7 @@ int onion_send_1(const Onion *onion, const uint8_t *plain, uint16_t len, IP_Port
    uint8_t ip_port[SIZE_IPPORT];
    ipport_pack(ip_port, &source);

-    uint8_t data[ONION_MAX_PACKET_SIZE];
+    uint8_t data[ONION_MAX_PACKET_SIZE] = {0};
    data[0] = NET_PACKET_ONION_SEND_1;
    memcpy(data + 1, nonce, CRYPTO_NONCE_SIZE);
    memcpy(data + 1 + CRYPTO_NONCE_SIZE, plain + SIZE_IPPORT, len - SIZE_IPPORT);
@ -411,7 +411,7 @@ static int handle_send_1(void *object, IP_Port source, const uint8_t *packet, ui
        return 1;
    }

-    uint8_t data[ONION_MAX_PACKET_SIZE];
+    uint8_t data[ONION_MAX_PACKET_SIZE] = {0};
    data[0] = NET_PACKET_ONION_SEND_2;
    memcpy(data + 1, packet + 1, CRYPTO_NONCE_SIZE);
    memcpy(data + 1 + CRYPTO_NONCE_SIZE, plain + SIZE_IPPORT, len - SIZE_IPPORT);
@ -478,7 +478,7 @@ static int handle_send_2(void *object, IP_Port source, const uint8_t *packet, ui
        return 1;
    }

-    uint8_t data[ONION_MAX_PACKET_SIZE];
+    uint8_t data[ONION_MAX_PACKET_SIZE] = {0};
    memcpy(data, plain + SIZE_IPPORT, len - SIZE_IPPORT);
    uint16_t data_len = len - SIZE_IPPORT;
    uint8_t *ret_part = data + (len - SIZE_IPPORT);
@ -537,7 +537,7 @@ static int handle_recv_3(void *object, IP_Port source, const uint8_t *packet, ui
        return 1;
    }

-    uint8_t data[ONION_MAX_PACKET_SIZE];
+    uint8_t data[ONION_MAX_PACKET_SIZE] = {0};
    data[0] = NET_PACKET_ONION_RECV_2;
    memcpy(data + 1, plain + SIZE_IPPORT, RETURN_2);
    memcpy(data + 1 + RETURN_2, packet + 1 + RETURN_3, length - (1 + RETURN_3));
@ -584,7 +584,7 @@ static int handle_recv_2(void *object, IP_Port source, const uint8_t *packet, ui
        return 1;
    }

-    uint8_t data[ONION_MAX_PACKET_SIZE];
+    uint8_t data[ONION_MAX_PACKET_SIZE] = {0};
    data[0] = NET_PACKET_ONION_RECV_1;
    memcpy(data + 1, plain + SIZE_IPPORT, RETURN_1);
    memcpy(data + 1 + RETURN_1, packet + 1 + RETURN_2, length - (1 + RETURN_2));
--- a/toxcore/onion_client.c
+++ b/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;

--- a/toxcore/ping.c
+++ b/toxcore/ping.c
@ -93,8 +93,7 @@ void ping_send_request(Ping *ping, IP_Port ipp, const uint8_t *public_key)
 static int ping_send_response(Ping *ping, IP_Port ipp, const uint8_t *public_key, uint64_t ping_id,
                              uint8_t *shared_encryption_key)
 {
-    uint8_t   pk[DHT_PING_SIZE];
-    int       rc;
+    uint8_t pk[DHT_PING_SIZE];

    if (id_equal(public_key, dht_get_self_public_key(ping->dht))) {
        return 1;
@ -109,10 +108,10 @@ static int ping_send_response(Ping *ping, IP_Port ipp, const uint8_t *public_key
    random_nonce(pk + 1 + CRYPTO_PUBLIC_KEY_SIZE); // Generate new nonce

    // Encrypt ping_id using recipient privkey
-    rc = encrypt_data_symmetric(shared_encryption_key,
-                                pk + 1 + CRYPTO_PUBLIC_KEY_SIZE,
-                                ping_plain, sizeof(ping_plain),
-                                pk + 1 + CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_NONCE_SIZE);
+    const int rc = encrypt_data_symmetric(shared_encryption_key,
+                                          pk + 1 + CRYPTO_PUBLIC_KEY_SIZE,
+                                          ping_plain, sizeof(ping_plain),
+                                          pk + 1 + CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_NONCE_SIZE);

    if (rc != PING_PLAIN_SIZE + CRYPTO_MAC_SIZE) {
        return 1;
@ -123,8 +122,7 @@ static int ping_send_response(Ping *ping, IP_Port ipp, const uint8_t *public_key

 static int handle_ping_request(void *object, IP_Port source, const uint8_t *packet, uint16_t length, void *userdata)
 {
-    DHT       *dht = (DHT *)object;
-    int        rc;
+    DHT *dht = (DHT *)object;

    if (length != DHT_PING_SIZE) {
        return 1;
@ -137,15 +135,15 @@ static int handle_ping_request(void *object, IP_Port source, const uint8_t *pack
    }

    uint8_t shared_key[CRYPTO_SHARED_KEY_SIZE];
-
    uint8_t ping_plain[PING_PLAIN_SIZE];
+
    // Decrypt ping_id
    dht_get_shared_key_recv(dht, shared_key, packet + 1);
-    rc = decrypt_data_symmetric(shared_key,
-                                packet + 1 + CRYPTO_PUBLIC_KEY_SIZE,
-                                packet + 1 + CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_NONCE_SIZE,
-                                PING_PLAIN_SIZE + CRYPTO_MAC_SIZE,
-                                ping_plain);
+    const int rc = decrypt_data_symmetric(shared_key,
+                                          packet + 1 + CRYPTO_PUBLIC_KEY_SIZE,
+                                          packet + 1 + CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_NONCE_SIZE,
+                                          PING_PLAIN_SIZE + CRYPTO_MAC_SIZE,
+                                          ping_plain);

    if (rc != sizeof(ping_plain)) {
        crypto_memzero(shared_key, sizeof(shared_key));
@ -157,7 +155,7 @@ static int handle_ping_request(void *object, IP_Port source, const uint8_t *pack
        return 1;
    }

-    uint64_t   ping_id;
+    uint64_t ping_id;
    memcpy(&ping_id, ping_plain + 1, sizeof(ping_id));
    // Send response
    ping_send_response(ping, source, packet + 1, ping_id, shared_key);
--- a/toxencryptsave/BUILD.bazel
+++ b/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__"],
 )