From 4a2cb37e4bc2749d275da7a7516905d93bd61fd6 Mon Sep 17 00:00:00 2001 From: iphydf Date: Thu, 13 Jan 2022 12:31:14 +0000 Subject: [PATCH] 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. --- .circleci/config.yml | 12 ++++ .cirrus.yml | 60 +++++++++++++------ .github/settings.yml | 1 - .github/workflows/ci.yml | 15 +++-- BUILD.bazel | 18 +++--- auto_tests/conference_av_test.c | 10 ++-- auto_tests/encryptsave_test.c | 6 +- other/astyle/format-source | 2 +- .../docker/tox-bootstrapd.sha256 | 2 +- toxav/BUILD.bazel | 13 ++-- toxcore/BUILD.bazel | 3 +- toxcore/DHT.c | 4 ++ toxcore/Messenger.c | 2 +- toxcore/crypto_core.c | 32 ++++++++-- toxcore/crypto_core_test.cc | 1 + toxcore/network.h | 18 ++++-- toxcore/onion.c | 10 ++-- toxcore/onion_client.c | 6 ++ toxcore/ping.c | 28 ++++----- toxencryptsave/BUILD.bazel | 3 +- 20 files changed, 162 insertions(+), 84 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 544c869e..f835ccdf 100644 --- 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: diff --git a/.cirrus.yml b/.cirrus.yml index dc06570d..56eb4ec0 100644 --- 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 diff --git a/.github/settings.yml b/.github/settings.yml index 3d8023f7..844bb061 100644 --- 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" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f7d5c490..497f668f 100644 --- 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 diff --git a/BUILD.bazel b/BUILD.bazel index 0b47b5da..750b3bf1 100644 --- 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 = [ diff --git a/auto_tests/conference_av_test.c b/auto_tests/conference_av_test.c index 24163440..b9a69aa2 100644 --- 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) { diff --git a/auto_tests/encryptsave_test.c b/auto_tests/encryptsave_test.c index c764b3d5..3387d544 100644 --- 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"); diff --git a/other/astyle/format-source b/other/astyle/format-source index 0fd5668a..cb8b8feb 100755 --- 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 diff --git a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 index 113eb8a1..8628c8d9 100644 --- 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 diff --git a/toxav/BUILD.bazel b/toxav/BUILD.bazel index 4fccacf5..015cfefb 100644 --- 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", diff --git a/toxcore/BUILD.bazel b/toxcore/BUILD.bazel index e11f91fc..2169cf63 100644 --- 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__"], ) diff --git a/toxcore/DHT.c b/toxcore/DHT.c index 05669410..c5e68a48 100644 --- 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); diff --git a/toxcore/Messenger.c b/toxcore/Messenger.c index 73ff0650..31573d70 100644 --- 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); diff --git a/toxcore/crypto_core.c b/toxcore/crypto_core.c index 3d8d0bad..b044ec5c 100644 --- 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 diff --git a/toxcore/crypto_core_test.cc b/toxcore/crypto_core_test.cc index 708bbe7a..faf53b42 100644 --- a/toxcore/crypto_core_test.cc +++ b/toxcore/crypto_core_test.cc @@ -3,6 +3,7 @@ #include #include +#include #include namespace { diff --git a/toxcore/network.h b/toxcore/network.h index b3b8da24..efc1988f 100644 --- 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 */ diff --git a/toxcore/onion.c b/toxcore/onion.c index 7dee92ac..6df46c03 100644 --- 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)); diff --git a/toxcore/onion_client.c b/toxcore/onion_client.c index 7d4cc0a5..3b6f0de2 100644 --- 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; diff --git a/toxcore/ping.c b/toxcore/ping.c index ea90428e..0eb52745 100644 --- 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); diff --git a/toxencryptsave/BUILD.bazel b/toxencryptsave/BUILD.bazel index 1b083c5f..bbd5e0ee 100644 --- 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__"], )