refactor: Add a bin_unpack_bin_max for max-length arrays.

These are statically allocated (e.g. `uint8_t[1024]`) arrays with
variable length data inside them. Examples are group topics and
nicknames.

.circleci/bazel-test

@@ -6,10 +6,7 @@ git submodule update --init --recursive
 /src/workspace/tools/inject-repo c-toxcore
 # TODO(iphydf): Re-enable fuzz-test when https://github.com/tweag/rules_nixpkgs/issues/442 is fixed.
 cd /src/workspace && bazel test -k \
-  --config=ci \
-  --config=remote \
   --build_tag_filters=-haskell,-fuzz-test \
   --test_tag_filters=-haskell,-fuzz-test \
   -- \
-  //c-toxcore/... \
   "$@"

.circleci/config.yml

@@ -7,11 +7,11 @@ workflows:
     jobs:
       # Dynamic analysis in the Bazel build
       - bazel-asan
+      - bazel-msan
       - bazel-tsan
       # Dynamic analysis with CMake
       - asan
       - tsan
-      - msan
       - ubsan
       # Static analysis
       - clang-analyze
@@ -29,6 +29,7 @@ jobs:
     steps:
       - checkout
       - run: .circleci/bazel-test
+          //c-toxcore/...
 
   bazel-tsan:
     working_directory: /tmp/cirrus-ci-build
@@ -38,11 +39,22 @@ jobs:
     steps:
       - checkout
       - run: .circleci/bazel-test
+          //c-toxcore/...
           -//c-toxcore/auto_tests:conference_av_test
           -//c-toxcore/auto_tests:conference_test
           -//c-toxcore/auto_tests:onion_test
           -//c-toxcore/auto_tests:tox_many_test
 
+  bazel-msan:
+    working_directory: /tmp/cirrus-ci-build
+    docker:
+      - image: toxchat/toktok-stack:latest-msan
+
+    steps:
+      - checkout
+      - run: .circleci/bazel-test
+          //c-toxcore/auto_tests:lossless_packet_test
+
   asan:
     working_directory: ~/work
     docker:
@@ -91,21 +103,6 @@ jobs:
       - run: git submodule update --init --recursive
       - run: CC=clang .circleci/cmake-ubsan
 
-  msan:
-    working_directory: ~/work
-    docker:
-      - image: toxchat/toktok-stack:latest-msan
-
-    steps:
-      - checkout
-      - run: git submodule update --init --recursive
-      - run: rm -rf /src/workspace/c-toxcore/* && mv * /src/workspace/c-toxcore/
-      - run:
-          cd /src/workspace && bazel test
-            //c-toxcore/auto_tests:lossless_packet_test
-            //c-toxcore/toxav/...
-            //c-toxcore/toxcore/...
-
   infer:
     working_directory: ~/work
     docker:

auto_tests/BUILD.bazel

@@ -29,12 +29,26 @@ flaky_tests = {
     "tox_many_tcp_test": True,
 }
 
+extra_args = {
+    "proxy_test": ["$(location //c-toxcore/other/proxy)"],
+}
+
+extra_data = {
+    "proxy_test": ["//c-toxcore/other/proxy"],
+}
+
 [cc_test(
     name = src[:-2],
     size = "small",
     srcs = [src],
-    args = ["$(location %s)" % src] + ["$(location //c-toxcore/other/proxy)"],
-    data = glob(["data/*"]) + ["//c-toxcore/other/proxy"],
+    args = ["$(location %s)" % src] + extra_args.get(
+        src[:-2],
+        [],
+    ),
+    data = glob(["data/*"]) + extra_data.get(
+        src[:-2],
+        [],
+    ),
     flaky = flaky_tests.get(
         src[:-2],
         False,

other/bootstrap_daemon/docker/tox-bootstrapd.sha256

@@ -1 +1 @@
-036adfc1e993624ae0bf49f08c2890bb44e6d4224a07a8c7fd2e2b5a8be6bf4c  /usr/local/bin/tox-bootstrapd
+c71f87c6ff30393d748bbdc118248eff90a4874cfa015b3113534f2333154555  /usr/local/bin/tox-bootstrapd

toxcore/Messenger.c

@@ -3220,9 +3220,13 @@ static State_Load_Status groups_load(Messenger *m, const uint8_t *data, uint32_t
 
         if (group_number < 0) {
             LOGGER_WARNING(m->log, "Failed to load group %u", i);
+            // Can't recover trivially. We may need to skip over some data here.
+            break;
         }
     }
 
+    LOGGER_DEBUG(m->log, "Successfully loaded %u groups", gc_count_groups(m->group_handler));
+
     bin_unpack_free(bu);
 
     return STATE_LOAD_STATUS_CONTINUE;

toxcore/bin_unpack.c

@@ -73,10 +73,14 @@ bool bin_unpack_array(Bin_Unpack *bu, uint32_t *size)
     return cmp_read_array(&bu->ctx, size) && *size <= bu->bytes_size;
 }
 
-bool bin_unpack_array_fixed(Bin_Unpack *bu, uint32_t required_size)
+bool bin_unpack_array_fixed(Bin_Unpack *bu, uint32_t required_size, uint32_t *actual_size)
 {
-    uint32_t size;
-    return cmp_read_array(&bu->ctx, &size) && size == required_size;
+    uint32_t size = 0;
+    const bool success = cmp_read_array(&bu->ctx, &size) && size == required_size;
+    if (actual_size != nullptr) {
+        *actual_size = size;
+    }
+    return success;
 }
 
 bool bin_unpack_bool(Bin_Unpack *bu, bool *val)
@@ -128,6 +132,18 @@ bool bin_unpack_bin(Bin_Unpack *bu, uint8_t **data_ptr, uint32_t *data_length_pt
     return true;
 }
 
+bool bin_unpack_bin_max(Bin_Unpack *bu, uint8_t *data, uint16_t *data_length_ptr, uint16_t max_data_length)
+{
+    uint32_t bin_size;
+    if (!bin_unpack_bin_size(bu, &bin_size) || bin_size > max_data_length) {
+        return false;
+    }
+
+    *data_length_ptr = bin_size;
+
+    return bin_unpack_bin_b(bu, data, bin_size);
+}
+
 bool bin_unpack_bin_fixed(Bin_Unpack *bu, uint8_t *data, uint32_t data_length)
 {
     uint32_t bin_size;

toxcore/bin_unpack.h

@@ -45,10 +45,14 @@ void bin_unpack_free(Bin_Unpack *bu);
 non_null() bool bin_unpack_array(Bin_Unpack *bu, uint32_t *size);
 
 /** @brief Start unpacking a fixed size MessagePack array.
+ *
+ * Fails if the array size is not the required size. If `actual_size` is passed a non-null
+ * pointer, the array size is written there.
  *
  * @retval false if the packed array size is not exactly the required size.
  */
-non_null() bool bin_unpack_array_fixed(Bin_Unpack *bu, uint32_t required_size);
+non_null(1) nullable(3)
+bool bin_unpack_array_fixed(Bin_Unpack *bu, uint32_t required_size, uint32_t *actual_size);
 
 /** @brief Unpack a MessagePack bool. */
 non_null() bool bin_unpack_bool(Bin_Unpack *bu, bool *val);
@@ -71,10 +75,16 @@ non_null() bool bin_unpack_nil(Bin_Unpack *bu);
  * large allocation unless the input array was already that large.
  */
 non_null() bool bin_unpack_bin(Bin_Unpack *bu, uint8_t **data_ptr, uint32_t *data_length_ptr);
+/** @brief Unpack a variable size MessagePack bin into a fixed size byte array.
+ *
+ * Stores unpacked data into `data` with its length stored in `data_length_ptr`. This function does
+ * not allocate memory and requires that `max_data_length` is less than or equal to `sizeof(arr)`
+ * when `arr` is passed as `data` pointer.
+ */
+non_null() bool bin_unpack_bin_max(Bin_Unpack *bu, uint8_t *data, uint16_t *data_length_ptr, uint16_t max_data_length);
 /** @brief Unpack a MessagePack bin of a fixed length into a pre-allocated byte array.
  *
- * Unlike the function above, this function does not allocate any memory, but requires the size to
- * be known up front.
+ * Similar to the function above, but doesn't output the data length.
  */
 non_null() bool bin_unpack_bin_fixed(Bin_Unpack *bu, uint8_t *data, uint32_t data_length);
 

toxcore/events/conference_invite.c

@@ -120,7 +120,7 @@ static bool tox_event_conference_invite_unpack(
     Tox_Event_Conference_Invite *event, Bin_Unpack *bu)
 {
     assert(event != nullptr);
-    if (!bin_unpack_array_fixed(bu, 3)) {
+    if (!bin_unpack_array_fixed(bu, 3, nullptr)) {
         return false;
     }
 

toxcore/events/conference_message.c

@@ -135,7 +135,7 @@ static bool tox_event_conference_message_unpack(
     Tox_Event_Conference_Message *event, Bin_Unpack *bu)
 {
     assert(event != nullptr);
-    if (!bin_unpack_array_fixed(bu, 4)) {
+    if (!bin_unpack_array_fixed(bu, 4, nullptr)) {
         return false;
     }
 

toxcore/events/conference_peer_name.c

@@ -120,7 +120,7 @@ static bool tox_event_conference_peer_name_unpack(
     Tox_Event_Conference_Peer_Name *event, Bin_Unpack *bu)
 {
     assert(event != nullptr);
-    if (!bin_unpack_array_fixed(bu, 3)) {
+    if (!bin_unpack_array_fixed(bu, 3, nullptr)) {
         return false;
     }
 

toxcore/events/conference_title.c

@@ -119,7 +119,7 @@ static bool tox_event_conference_title_unpack(
     Tox_Event_Conference_Title *event, Bin_Unpack *bu)
 {
     assert(event != nullptr);
-    if (!bin_unpack_array_fixed(bu, 3)) {
+    if (!bin_unpack_array_fixed(bu, 3, nullptr)) {
         return false;
     }
 

toxcore/events/file_chunk_request.c

@@ -112,7 +112,7 @@ static bool tox_event_file_chunk_request_unpack(
     Tox_Event_File_Chunk_Request *event, Bin_Unpack *bu)
 {
     assert(event != nullptr);
-    if (!bin_unpack_array_fixed(bu, 4)) {
+    if (!bin_unpack_array_fixed(bu, 4, nullptr)) {
         return false;
     }
 

toxcore/events/file_recv.c

@@ -149,7 +149,7 @@ static bool tox_event_file_recv_unpack(
     Tox_Event_File_Recv *event, Bin_Unpack *bu)
 {
     assert(event != nullptr);
-    if (!bin_unpack_array_fixed(bu, 5)) {
+    if (!bin_unpack_array_fixed(bu, 5, nullptr)) {
         return false;
     }
 

toxcore/events/file_recv_chunk.c

@@ -134,7 +134,7 @@ static bool tox_event_file_recv_chunk_unpack(
     Tox_Event_File_Recv_Chunk *event, Bin_Unpack *bu)
 {
     assert(event != nullptr);
-    if (!bin_unpack_array_fixed(bu, 4)) {
+    if (!bin_unpack_array_fixed(bu, 4, nullptr)) {
         return false;
     }
 

toxcore/events/file_recv_control.c

@@ -99,7 +99,7 @@ static bool tox_event_file_recv_control_unpack(
     Tox_Event_File_Recv_Control *event, Bin_Unpack *bu)
 {
     assert(event != nullptr);
-    if (!bin_unpack_array_fixed(bu, 3)) {
+    if (!bin_unpack_array_fixed(bu, 3, nullptr)) {
         return false;
     }
 

toxcore/events/friend_connection_status.c

@@ -86,7 +86,7 @@ static bool tox_event_friend_connection_status_unpack(
     Tox_Event_Friend_Connection_Status *event, Bin_Unpack *bu)
 {
     assert(event != nullptr);
-    if (!bin_unpack_array_fixed(bu, 2)) {
+    if (!bin_unpack_array_fixed(bu, 2, nullptr)) {
         return false;
     }
 

toxcore/events/friend_lossless_packet.c

@@ -105,7 +105,7 @@ static bool tox_event_friend_lossless_packet_unpack(
     Tox_Event_Friend_Lossless_Packet *event, Bin_Unpack *bu)
 {
     assert(event != nullptr);
-    if (!bin_unpack_array_fixed(bu, 2)) {
+    if (!bin_unpack_array_fixed(bu, 2, nullptr)) {
         return false;
     }
 

toxcore/events/friend_lossy_packet.c

@@ -104,7 +104,7 @@ static bool tox_event_friend_lossy_packet_unpack(
     Tox_Event_Friend_Lossy_Packet *event, Bin_Unpack *bu)
 {
     assert(event != nullptr);
-    if (!bin_unpack_array_fixed(bu, 2)) {
+    if (!bin_unpack_array_fixed(bu, 2, nullptr)) {
         return false;
     }
 

toxcore/events/friend_message.c

@@ -119,7 +119,7 @@ static bool tox_event_friend_message_unpack(
     Tox_Event_Friend_Message *event, Bin_Unpack *bu)
 {
     assert(event != nullptr);
-    if (!bin_unpack_array_fixed(bu, 3)) {
+    if (!bin_unpack_array_fixed(bu, 3, nullptr)) {
         return false;
     }
 

toxcore/events/friend_name.c

@@ -104,7 +104,7 @@ static bool tox_event_friend_name_unpack(
     Tox_Event_Friend_Name *event, Bin_Unpack *bu)
 {
     assert(event != nullptr);
-    if (!bin_unpack_array_fixed(bu, 2)) {
+    if (!bin_unpack_array_fixed(bu, 2, nullptr)) {
         return false;
     }
 

toxcore/events/friend_read_receipt.c

@@ -83,7 +83,7 @@ static bool tox_event_friend_read_receipt_unpack(
     Tox_Event_Friend_Read_Receipt *event, Bin_Unpack *bu)
 {
     assert(event != nullptr);
-    if (!bin_unpack_array_fixed(bu, 2)) {
+    if (!bin_unpack_array_fixed(bu, 2, nullptr)) {
         return false;
     }
 

toxcore/events/friend_request.c

@@ -105,7 +105,7 @@ static bool tox_event_friend_request_unpack(
     Tox_Event_Friend_Request *event, Bin_Unpack *bu)
 {
     assert(event != nullptr);
-    if (!bin_unpack_array_fixed(bu, 2)) {
+    if (!bin_unpack_array_fixed(bu, 2, nullptr)) {
         return false;
     }
 

toxcore/events/friend_status.c

@@ -84,7 +84,7 @@ static bool tox_event_friend_status_unpack(
     Tox_Event_Friend_Status *event, Bin_Unpack *bu)
 {
     assert(event != nullptr);
-    if (!bin_unpack_array_fixed(bu, 2)) {
+    if (!bin_unpack_array_fixed(bu, 2, nullptr)) {
         return false;
     }
 

toxcore/events/friend_status_message.c

@@ -106,7 +106,7 @@ static bool tox_event_friend_status_message_unpack(
     Tox_Event_Friend_Status_Message *event, Bin_Unpack *bu)
 {
     assert(event != nullptr);
-    if (!bin_unpack_array_fixed(bu, 2)) {
+    if (!bin_unpack_array_fixed(bu, 2, nullptr)) {
         return false;
     }
 

toxcore/events/friend_typing.c

@@ -82,7 +82,7 @@ static bool tox_event_friend_typing_unpack(
     Tox_Event_Friend_Typing *event, Bin_Unpack *bu)
 {
     assert(event != nullptr);
-    if (!bin_unpack_array_fixed(bu, 2)) {
+    if (!bin_unpack_array_fixed(bu, 2, nullptr)) {
         return false;
     }
 

toxcore/group_pack.c

@@ -22,7 +22,7 @@
 non_null()
 static bool load_unpack_state_values(GC_Chat *chat, Bin_Unpack *bu)
 {
-    if (!bin_unpack_array_fixed(bu, 8)) {
+    if (!bin_unpack_array_fixed(bu, 8, nullptr)) {
         LOGGER_ERROR(chat->log, "Group state values array malformed");
         return false;
     }
@@ -58,15 +58,23 @@ static bool load_unpack_state_values(GC_Chat *chat, Bin_Unpack *bu)
 non_null()
 static bool load_unpack_state_bin(GC_Chat *chat, Bin_Unpack *bu)
 {
-    if (!bin_unpack_array_fixed(bu, 5)) {
+    if (!bin_unpack_array_fixed(bu, 5, nullptr)) {
         LOGGER_ERROR(chat->log, "Group state binary array malformed");
         return false;
     }
 
-    if (!(bin_unpack_bin_fixed(bu, chat->shared_state_sig, SIGNATURE_SIZE)
-            && bin_unpack_bin_fixed(bu, chat->shared_state.founder_public_key, EXT_PUBLIC_KEY_SIZE)
-            && bin_unpack_bin_fixed(bu, chat->shared_state.group_name, chat->shared_state.group_name_len)
-            && bin_unpack_bin_fixed(bu, chat->shared_state.password, chat->shared_state.password_length)
+    if (!bin_unpack_bin_fixed(bu, chat->shared_state_sig, SIGNATURE_SIZE)) {
+        LOGGER_ERROR(chat->log, "Failed to unpack shared state signature");
+        return false;
+    }
+
+    if (!bin_unpack_bin_fixed(bu, chat->shared_state.founder_public_key, EXT_PUBLIC_KEY_SIZE)) {
+        LOGGER_ERROR(chat->log, "Failed to unpack founder public key");
+        return false;
+    }
+
+    if (!(bin_unpack_bin_max(bu, chat->shared_state.group_name, &chat->shared_state.group_name_len, sizeof(chat->shared_state.group_name))
+            && bin_unpack_bin_max(bu, chat->shared_state.password, &chat->shared_state.password_length, sizeof(chat->shared_state.password))
             && bin_unpack_bin_fixed(bu, chat->shared_state.mod_list_hash, MOD_MODERATION_HASH_SIZE))) {
         LOGGER_ERROR(chat->log, "Failed to unpack state binary data");
         return false;
@@ -78,7 +86,7 @@ static bool load_unpack_state_bin(GC_Chat *chat, Bin_Unpack *bu)
 non_null()
 static bool load_unpack_topic_info(GC_Chat *chat, Bin_Unpack *bu)
 {
-    if (!bin_unpack_array_fixed(bu, 6)) {
+    if (!bin_unpack_array_fixed(bu, 6, nullptr)) {
         LOGGER_ERROR(chat->log, "Group topic array malformed");
         return false;
     }
@@ -86,7 +94,7 @@ static bool load_unpack_topic_info(GC_Chat *chat, Bin_Unpack *bu)
     if (!(bin_unpack_u32(bu, &chat->topic_info.version)
             && bin_unpack_u16(bu, &chat->topic_info.length)
             && bin_unpack_u16(bu, &chat->topic_info.checksum)
-            && bin_unpack_bin_fixed(bu, chat->topic_info.topic, chat->topic_info.length)
+            && bin_unpack_bin_max(bu, chat->topic_info.topic, &chat->topic_info.length, sizeof(chat->topic_info.topic))
             && bin_unpack_bin_fixed(bu, chat->topic_info.public_sig_key, SIG_PUBLIC_KEY_SIZE)
             && bin_unpack_bin_fixed(bu, chat->topic_sig, SIGNATURE_SIZE))) {
         LOGGER_ERROR(chat->log, "Failed to unpack topic info");
@@ -99,8 +107,9 @@ static bool load_unpack_topic_info(GC_Chat *chat, Bin_Unpack *bu)
 non_null()
 static bool load_unpack_mod_list(GC_Chat *chat, Bin_Unpack *bu)
 {
-    if (!bin_unpack_array_fixed(bu, 2)) {
-        LOGGER_ERROR(chat->log, "Group mod list array malformed");
+    uint32_t actual_size = 0;
+    if (!bin_unpack_array_fixed(bu, 2, &actual_size)) {
+        LOGGER_ERROR(chat->log, "Group mod list array malformed: %d != 2", actual_size);
         return false;
     }
 
@@ -148,7 +157,7 @@ static bool load_unpack_mod_list(GC_Chat *chat, Bin_Unpack *bu)
 non_null()
 static bool load_unpack_keys(GC_Chat *chat, Bin_Unpack *bu)
 {
-    if (!bin_unpack_array_fixed(bu, 4)) {
+    if (!bin_unpack_array_fixed(bu, 4, nullptr)) {
         LOGGER_ERROR(chat->log, "Group keys array malformed");
         return false;
     }
@@ -167,7 +176,7 @@ static bool load_unpack_keys(GC_Chat *chat, Bin_Unpack *bu)
 non_null()
 static bool load_unpack_self_info(GC_Chat *chat, Bin_Unpack *bu)
 {
-    if (!bin_unpack_array_fixed(bu, 4)) {
+    if (!bin_unpack_array_fixed(bu, 4, nullptr)) {
         LOGGER_ERROR(chat->log, "Group self info array malformed");
         return false;
     }
@@ -214,7 +223,7 @@ static bool load_unpack_self_info(GC_Chat *chat, Bin_Unpack *bu)
 non_null()
 static bool load_unpack_saved_peers(GC_Chat *chat, Bin_Unpack *bu)
 {
-    if (!bin_unpack_array_fixed(bu, 2)) {
+    if (!bin_unpack_array_fixed(bu, 2, nullptr)) {
         LOGGER_ERROR(chat->log, "Group saved peers array malformed");
         return false;
     }
@@ -256,8 +265,9 @@ static bool load_unpack_saved_peers(GC_Chat *chat, Bin_Unpack *bu)
 
 bool gc_load_unpack_group(GC_Chat *chat, Bin_Unpack *bu)
 {
-    if (!bin_unpack_array_fixed(bu, 7)) {
-        LOGGER_ERROR(chat->log, "Group info array malformed");
+    uint32_t actual_size;
+    if (!bin_unpack_array_fixed(bu, 7, &actual_size)) {
+        LOGGER_ERROR(chat->log, "Group info array malformed: %d != 7", actual_size);
         return false;
     }
 

toxcore/network.c

@@ -1086,7 +1086,7 @@ void networking_poll(const Networking_Core *net, void *userdata)
     }
 
     IP_Port ip_port;
-    uint8_t data[MAX_UDP_PACKET_SIZE];
+    uint8_t data[MAX_UDP_PACKET_SIZE] = {0};
     uint32_t length;
 
     while (receivepacket(net->ns, net->mem, net->log, net->sock, &ip_port, data, &length) != -1) {