cleanup: Don't use VLAs for huge allocations.

One of these was creating a single 262144 byte stack frame. We now have
a way to check and limit the allocation size of a VLA. The `Cmp_Data`
ones were also fairly large. Now, no allocation is larger than 2KiB
(though rtp.c allocates close to that much).

auto_tests/encryptsave_test.c

@@ -59,10 +59,12 @@ static void test_save_friend(void)
     ck_assert_msg(test != UINT32_MAX, "Failed to add friend");
 
     size_t size = tox_get_savedata_size(tox1);
-    VLA(uint8_t, data, size);
+    uint8_t *data = (uint8_t *)malloc(size);
+    ck_assert(data != nullptr);
     tox_get_savedata(tox1, data);
     size_t size2 = size + TOX_PASS_ENCRYPTION_EXTRA_LENGTH;
-    VLA(uint8_t, enc_data, size2);
+    uint8_t *enc_data = (uint8_t *)malloc(size2);
+    ck_assert(enc_data != nullptr);
     Tox_Err_Encryption error1;
     bool ret = tox_pass_encrypt(data, size, (const uint8_t *)"correcthorsebatterystaple", 25, enc_data, &error1);
     ck_assert_msg(ret, "failed to encrypted save: %d", error1);
@@ -78,7 +80,8 @@ static void test_save_friend(void)
     ck_assert_msg(err2 == TOX_ERR_NEW_LOAD_ENCRYPTED, "wrong error! %d. should fail with %d", err2,
                   TOX_ERR_NEW_LOAD_ENCRYPTED);
     ck_assert_msg(tox3 == nullptr, "tox_new with error should return NULL");
-    VLA(uint8_t, dec_data, size);
+    uint8_t *dec_data = (uint8_t *)malloc(size);
+    ck_assert(dec_data != nullptr);
     Tox_Err_Decryption err3;
     ret = tox_pass_decrypt(enc_data, size2, (const uint8_t *)"correcthorsebatterystaple", 25, dec_data, &err3);
     ck_assert_msg(ret, "failed to decrypt save: %d", err3);
@@ -91,21 +94,25 @@ static void test_save_friend(void)
     ck_assert_msg(memcmp(address, address2, TOX_PUBLIC_KEY_SIZE) == 0, "addresses don't match!");
 
     size = tox_get_savedata_size(tox3);
-    VLA(uint8_t, data2, size);
+    uint8_t *data2 = (uint8_t *)malloc(size);
+    ck_assert(data2 != nullptr);
     tox_get_savedata(tox3, data2);
     Tox_Err_Key_Derivation keyerr;
     Tox_Pass_Key *key = tox_pass_key_derive((const uint8_t *)"123qweasdzxc", 12, &keyerr);
     ck_assert_msg(key != nullptr, "pass key allocation failure");
     memcpy((uint8_t *)key, test_salt, TOX_PASS_SALT_LENGTH);
     memcpy((uint8_t *)key + TOX_PASS_SALT_LENGTH, known_key2, TOX_PASS_KEY_LENGTH);
     size2 = size + TOX_PASS_ENCRYPTION_EXTRA_LENGTH;
-    VLA(uint8_t, encdata2, size2);
+    uint8_t *encdata2 = (uint8_t *)malloc(size2);
+    ck_assert(encdata2 != nullptr);
     ret = tox_pass_key_encrypt(key, data2, size, encdata2, &error1);
     ck_assert_msg(ret, "failed to key encrypt %d", error1);
     ck_assert_msg(tox_is_data_encrypted(encdata2), "magic number the second missing");
 
-    VLA(uint8_t, out1, size);
-    VLA(uint8_t, out2, size);
+    uint8_t *out1 = (uint8_t *)malloc(size);
+    ck_assert(out1 != nullptr);
+    uint8_t *out2 = (uint8_t *)malloc(size);
+    ck_assert(out2 != nullptr);
     ret = tox_pass_decrypt(encdata2, size2, (const uint8_t *)pw, pwlen, out1, &err3);
     ck_assert_msg(ret, "failed to pw decrypt %d", err3);
     ret = tox_pass_key_decrypt(key, encdata2, size2, out2, &err3);
@@ -129,6 +136,14 @@ static void test_save_friend(void)
     tox_kill(tox2);
     tox_kill(tox3);
     tox_kill(tox4);
+
+    free(out2);
+    free(out1);
+    free(encdata2);
+    free(data2);
+    free(dec_data);
+    free(enc_data);
+    free(data);
 }
 
 static void test_keys(void)

auto_tests/save_friend_test.c

@@ -108,7 +108,7 @@ int main(void)
     }
 
     size_t save_size = tox_get_savedata_size(tox1);
-    VLA(uint8_t, savedata, save_size);
+    uint8_t *savedata = (uint8_t *)malloc(save_size);
     tox_get_savedata(tox1, savedata);
 
     struct Tox_Options *const options = tox_options_new(nullptr);
@@ -129,6 +129,7 @@ int main(void)
     tox_kill(tox1);
     tox_kill(tox2);
     tox_kill(tox_to_compare);
+    free(savedata);
 
     return 0;
 }

auto_tests/tox_one_test.c

@@ -76,7 +76,7 @@ static void test_one(void)
 
     tox_self_get_address(tox1, address);
     size_t save_size = tox_get_savedata_size(tox1);
-    VLA(uint8_t, data, save_size);
+    uint8_t *data = (uint8_t *)malloc(save_size);
     tox_get_savedata(tox1, data);
 
     tox_kill(tox2);
@@ -126,6 +126,7 @@ static void test_one(void)
     tox_options_free(options);
     tox_kill(tox1);
     tox_kill(tox2);
+    free(data);
 }
 
 

other/bootstrap_daemon/docker/tox-bootstrapd.sha256

@@ -1 +1 @@
-502cc22df74fa369b2c09d117176705a1e801726db6f8360c688aee90973fa22  /usr/local/bin/tox-bootstrapd
+ffc13de36541eb75974c274f21c8474f396476122e63d3ce7f8cdf2c44bea095  /usr/local/bin/tox-bootstrapd

toxcore/DHT.c

@@ -475,14 +475,19 @@ static int dht_create_packet(const uint8_t public_key[CRYPTO_PUBLIC_KEY_SIZE],
                              const uint8_t *shared_key, const uint8_t type,
                              const uint8_t *plain, size_t plain_length, uint8_t *packet)
 {
-    VLA(uint8_t, encrypted, plain_length + CRYPTO_MAC_SIZE);
+    uint8_t *encrypted = (uint8_t *)malloc(plain_length + CRYPTO_MAC_SIZE);
     uint8_t nonce[CRYPTO_NONCE_SIZE];
 
+    if (encrypted == nullptr) {
+        return -1;
+    }
+
     random_nonce(nonce);
 
     const int encrypted_length = encrypt_data_symmetric(shared_key, nonce, plain, plain_length, encrypted);
 
     if (encrypted_length == -1) {
+        free(encrypted);
         return -1;
     }
 
@@ -491,6 +496,7 @@ static int dht_create_packet(const uint8_t public_key[CRYPTO_PUBLIC_KEY_SIZE],
     memcpy(packet + 1 + CRYPTO_PUBLIC_KEY_SIZE, nonce, CRYPTO_NONCE_SIZE);
     memcpy(packet + 1 + CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_NONCE_SIZE, encrypted, encrypted_length);
 
+    free(encrypted);
     return 1 + CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_NONCE_SIZE + encrypted_length;
 }
 
@@ -968,7 +974,11 @@ static void sort_client_list(Client_data *list, uint64_t cur_time, unsigned int
 {
     // Pass comp_public_key to qsort with each Client_data entry, so the
     // comparison function can use it as the base of comparison.
-    VLA(DHT_Cmp_Data, cmp_list, length);
+    DHT_Cmp_Data *cmp_list = (DHT_Cmp_Data *)calloc(length, sizeof(DHT_Cmp_Data));
+
+    if (cmp_list == nullptr) {
+        return;
+    }
 
     for (uint32_t i = 0; i < length; ++i) {
         cmp_list[i].cur_time = cur_time;
@@ -981,6 +991,8 @@ static void sort_client_list(Client_data *list, uint64_t cur_time, unsigned int
     for (uint32_t i = 0; i < length; ++i) {
         list[i] = cmp_list[i].entry;
     }
+
+    free(cmp_list);
 }
 
 non_null()
@@ -1699,11 +1711,17 @@ static uint8_t do_ping_and_sendnode_requests(DHT *dht, uint64_t *lastgetnode, co
     const uint64_t temp_time = mono_time_get(dht->mono_time);
 
     uint32_t num_nodes = 0;
-    VLA(Client_data *, client_list, list_count * 2);
-    VLA(IPPTsPng *, assoc_list, list_count * 2);
+    Client_data **client_list = (Client_data **)calloc(list_count * 2, sizeof(Client_data *));
+    IPPTsPng **assoc_list = (IPPTsPng **)calloc(list_count * 2, sizeof(IPPTsPng *));
     unsigned int sort = 0;
     bool sort_ok = false;
 
+    if (client_list == nullptr || assoc_list == nullptr) {
+        free(assoc_list);
+        free(client_list);
+        return 0;
+    }
+
     for (uint32_t i = 0; i < list_count; ++i) {
         /* If node is not dead. */
         Client_data *client = &list[i];
@@ -1757,6 +1775,8 @@ static uint8_t do_ping_and_sendnode_requests(DHT *dht, uint64_t *lastgetnode, co
         ++*bootstrap_times;
     }
 
+    free(assoc_list);
+    free(client_list);
     return not_kill;
 }
 

toxcore/ccompat.h

@@ -27,7 +27,7 @@
 // you may run out of stack space.
 #if !defined(DISABLE_VLA) && !defined(_MSC_VER) && defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
 // C99 VLAs.
-#define VLA(type, name, size) type name[size]
+#define ALLOC_VLA(type, name, size) type name[size]
 #define SIZEOF_VLA sizeof
 #else
 
@@ -48,13 +48,22 @@
 #endif
 #endif
 
-#define VLA(type, name, size)                           \
+#define ALLOC_VLA(type, name, size)                     \
   const size_t name##_vla_size = (size) * sizeof(type); \
   type *const name = (type *)alloca(name##_vla_size)
 #define SIZEOF_VLA(name) name##_vla_size
 
 #endif
 
+#ifdef MAX_VLA_SIZE
+#include <assert.h>
+#define VLA(type, name, size) \
+  ALLOC_VLA(type, name, size); \
+  assert((size_t)(size) * sizeof(type) <= MAX_VLA_SIZE)
+#else
+#define VLA ALLOC_VLA
+#endif
+
 #if !defined(__cplusplus) || __cplusplus < 201103L
 #define nullptr NULL
 #ifndef static_assert

toxcore/onion_announce.c

@@ -310,7 +310,11 @@ static void sort_onion_announce_list(Onion_Announce_Entry *list, unsigned int le
 {
     // Pass comp_public_key to qsort with each Client_data entry, so the
     // comparison function can use it as the base of comparison.
-    VLA(Cmp_Data, cmp_list, length);
+    Cmp_Data *cmp_list = (Cmp_Data *)calloc(length, sizeof(Cmp_Data));
+
+    if (cmp_list == nullptr) {
+        return;
+    }
 
     for (uint32_t i = 0; i < length; ++i) {
         cmp_list[i].mono_time = mono_time;
@@ -323,6 +327,8 @@ static void sort_onion_announce_list(Onion_Announce_Entry *list, unsigned int le
     for (uint32_t i = 0; i < length; ++i) {
         list[i] = cmp_list[i].entry;
     }
+
+    free(cmp_list);
 }
 
 /** add entry to entries list

toxcore/onion_client.c

@@ -660,7 +660,11 @@ static void sort_onion_node_list(Onion_Node *list, unsigned int length, const Mo
 {
     // Pass comp_public_key to qsort with each Client_data entry, so the
     // comparison function can use it as the base of comparison.
-    VLA(Onion_Client_Cmp_Data, cmp_list, length);
+    Onion_Client_Cmp_Data *cmp_list = (Onion_Client_Cmp_Data *)calloc(length, sizeof(Onion_Client_Cmp_Data));
+
+    if (cmp_list == nullptr) {
+        return;
+    }
 
     for (uint32_t i = 0; i < length; ++i) {
         cmp_list[i].mono_time = mono_time;
@@ -673,6 +677,8 @@ static void sort_onion_node_list(Onion_Node *list, unsigned int length, const Mo
     for (uint32_t i = 0; i < length; ++i) {
         list[i] = cmp_list[i].entry;
     }
+
+    free(cmp_list);
 }
 
 non_null()