Fix group custom packet size limits

Lossy custom packets cannot be split, therefore they need to be limited
to the maximum safe UDP packet size.

Author: JFreegman

other/bootstrap_daemon/docker/tox-bootstrapd.sha256

@@ -1 +1 @@
-769233afaac07be03c094411e6ec8f031bde41beae475c74e154e51e51e9168b  /usr/local/bin/tox-bootstrapd
+cbd8bea9d23a961f27aacd35c4509fc1c22d72356f61d1ec74cc469b6b14490d  /usr/local/bin/tox-bootstrapd

toxcore/group_chats.c

@@ -4893,10 +4893,26 @@ static int handle_gc_private_message(const GC_Session *c, const GC_Chat *chat, c
     return 0;
 }
 
+/** @brief Returns false if a custom packet is too large. */
+static bool custom_gc_packet_length_is_valid(uint16_t length, bool lossless)
+{
+    if (lossless) {
+        if (length > MAX_GC_CUSTOM_LOSSLESS_PACKET_SIZE) {
+            return false;
+        }
+    } else {
+        if (length > MAX_GC_CUSTOM_LOSSY_PACKET_SIZE) {
+            return false;
+        }
+    }
+
+    return true;
+}
+
 int gc_send_custom_private_packet(const GC_Chat *chat, bool lossless, uint32_t peer_id, const uint8_t *message,
                                   uint16_t length)
 {
-    if (length > MAX_GC_CUSTOM_PACKET_SIZE) {
+    if (!custom_gc_packet_length_is_valid(length, lossless)) {
         return -1;
     }
 
@@ -4926,16 +4942,23 @@ int gc_send_custom_private_packet(const GC_Chat *chat, bool lossless, uint32_t p
 
     return ret ? 0 : -5;
 }
+
+
+
 /** @brief Handles a custom private packet.
  *
  * @retval 0 if packet is handled correctly.
  * @retval -1 if packet has invalid size.
  */
-non_null(1, 2, 3, 4) nullable(6)
+non_null(1, 2, 3, 4) nullable(7)
 static int handle_gc_custom_private_packet(const GC_Session *c, const GC_Chat *chat, const GC_Peer *peer,
-        const uint8_t *data, uint16_t length, void *userdata)
+        const uint8_t *data, uint16_t length, bool lossless, void *userdata)
 {
-    if (data == nullptr || length == 0 || length > MAX_GC_CUSTOM_PACKET_SIZE) {
+    if (!custom_gc_packet_length_is_valid(length, lossless)) {
+        return -1;
+    }
+
+    if (data == nullptr || length == 0) {
         return -1;
     }
 
@@ -4952,7 +4975,7 @@ static int handle_gc_custom_private_packet(const GC_Session *c, const GC_Chat *c
 
 int gc_send_custom_packet(const GC_Chat *chat, bool lossless, const uint8_t *data, uint16_t length)
 {
-    if (length > MAX_GC_CUSTOM_PACKET_SIZE) {
+    if (!custom_gc_packet_length_is_valid(length, lossless)) {
         return -1;
     }
 
@@ -4978,11 +5001,15 @@ int gc_send_custom_packet(const GC_Chat *chat, bool lossless, const uint8_t *dat
  * Return 0 if packet is handled correctly.
  * Return -1 if packet has invalid size.
  */
-non_null(1, 2, 3, 4) nullable(6)
+non_null(1, 2, 3, 4) nullable(7)
 static int handle_gc_custom_packet(const GC_Session *c, const GC_Chat *chat, const GC_Peer *peer, const uint8_t *data,
-                                   uint16_t length, void *userdata)
+                                   uint16_t length, bool lossless, void *userdata)
 {
-    if (data == nullptr || length == 0 || length > MAX_GC_CUSTOM_PACKET_SIZE) {
+    if (!custom_gc_packet_length_is_valid(length, lossless)) {
+        return -1;
+    }
+
+    if (data == nullptr || length == 0) {
         return -1;
     }
 
@@ -5913,12 +5940,12 @@ bool handle_gc_lossless_helper(const GC_Session *c, GC_Chat *chat, uint32_t peer
         }
 
         case GP_CUSTOM_PACKET: {
-            ret = handle_gc_custom_packet(c, chat, peer, data, length, userdata);
+            ret = handle_gc_custom_packet(c, chat, peer, data, length, true, userdata);
             break;
         }
 
         case GP_CUSTOM_PRIVATE_PACKET: {
-            ret = handle_gc_custom_private_packet(c, chat, peer, data, length, userdata);
+            ret = handle_gc_custom_private_packet(c, chat, peer, data, length, true, userdata);
             break;
         }
 
@@ -6168,12 +6195,12 @@ static bool handle_gc_lossy_packet(const GC_Session *c, GC_Chat *chat, const uin
         }
 
         case GP_CUSTOM_PACKET: {
-            ret = handle_gc_custom_packet(c, chat, peer, data, payload_len, userdata);
+            ret = handle_gc_custom_packet(c, chat, peer, data, payload_len, false, userdata);
             break;
         }
 
         case GP_CUSTOM_PRIVATE_PACKET: {
-            ret = handle_gc_custom_private_packet(c, chat, peer, data, payload_len, userdata);
+            ret = handle_gc_custom_private_packet(c, chat, peer, data, payload_len, false, userdata);
             break;
         }
 

toxcore/group_common.h

@@ -23,19 +23,23 @@
 #define GC_MESSAGE_PSEUDO_ID_SIZE 4
 #define GROUP_MAX_MESSAGE_LENGTH  1368
 
-#define MAX_GC_MESSAGE_SIZE       GROUP_MAX_MESSAGE_LENGTH
-#define MAX_GC_MESSAGE_RAW_SIZE   (MAX_GC_MESSAGE_SIZE + GC_MESSAGE_PSEUDO_ID_SIZE)
-#define MAX_GC_CUSTOM_PACKET_SIZE 1373
+/* Max size of a packet chunk. Packets larger than this must be split up.
+ *
+ * For an explanation on why this value was chosen, see the following link: https://archive.ph/vsCOG
+ */
+#define MAX_GC_PACKET_CHUNK_SIZE 500
+
+#define MAX_GC_MESSAGE_SIZE GROUP_MAX_MESSAGE_LENGTH
+#define MAX_GC_MESSAGE_RAW_SIZE (MAX_GC_MESSAGE_SIZE + GC_MESSAGE_PSEUDO_ID_SIZE)
+#define MAX_GC_CUSTOM_LOSSLESS_PACKET_SIZE 1373
+#define MAX_GC_CUSTOM_LOSSY_PACKET_SIZE MAX_GC_PACKET_CHUNK_SIZE
 #define MAX_GC_PASSWORD_SIZE 32
 #define MAX_GC_SAVED_INVITES 10
 #define MAX_GC_PEERS_DEFAULT 100
 #define MAX_GC_SAVED_TIMEOUTS 12
 #define GC_MAX_SAVED_PEERS 100
 #define GC_SAVED_PEER_SIZE (ENC_PUBLIC_KEY_SIZE + sizeof(Node_format) + sizeof(IP_Port))
 
-/* Max size of a packet chunk. Packets larger than this must be split up. */
-#define MAX_GC_PACKET_CHUNK_SIZE 500
-
 /* Max size of a complete encrypted packet including headers. */
 #define MAX_GC_PACKET_SIZE (MAX_GC_PACKET_CHUNK_SIZE * 100)
 

toxcore/tox.c

@@ -58,8 +58,8 @@ static_assert(TOX_MAX_STATUS_MESSAGE_LENGTH == MAX_STATUSMESSAGE_LENGTH,
               "TOX_MAX_STATUS_MESSAGE_LENGTH is assumed to be equal to MAX_STATUSMESSAGE_LENGTH");
 static_assert(TOX_GROUP_MAX_MESSAGE_LENGTH == GROUP_MAX_MESSAGE_LENGTH,
               "TOX_GROUP_MAX_MESSAGE_LENGTH is assumed to be equal to GROUP_MAX_MESSAGE_LENGTH");
-static_assert(TOX_MAX_CUSTOM_PACKET_SIZE == MAX_GC_CUSTOM_PACKET_SIZE,
-              "TOX_MAX_CUSTOM_PACKET_SIZE is assumed to be equal to MAX_GC_CUSTOM_PACKET_SIZE");
+static_assert(TOX_MAX_CUSTOM_PACKET_SIZE == MAX_GC_CUSTOM_LOSSLESS_PACKET_SIZE,
+              "TOX_MAX_CUSTOM_PACKET_SIZE is assumed to be equal to MAX_GC_CUSTOM_LOSSLESS_PACKET_SIZE");
 
 struct Tox_Userdata {
     Tox *tox;

toxcore/tox.h

@@ -3303,6 +3303,16 @@ uint32_t tox_group_max_part_length(void);
  */
 #define TOX_GROUP_MAX_MESSAGE_LENGTH    1368
 
+/**
+ * Maximum length of a group custom lossy packet.
+ */
+#define TOX_GROUP_MAX_CUSTOM_LOSSY_PACKET_LENGTH 500
+
+/**
+ * Maximum length of a group custom lossless packet.
+ */
+#define TOX_GROUP_MAX_CUSTOM_LOSSLESS_PACKET_LENGTH 1373
+
 /**
  * Maximum length of a group name.
  */
@@ -4477,7 +4487,9 @@ typedef enum Tox_Err_Group_Send_Custom_Packet {
     TOX_ERR_GROUP_SEND_CUSTOM_PACKET_GROUP_NOT_FOUND,
 
     /**
-     * Message length exceeded TOX_GROUP_MAX_MESSAGE_LENGTH.
+     * Message length exceeded TOX_GROUP_MAX_CUSTOM_LOSSY_PACKET_LENGTH if the
+     * packet was lossy, or TOX_GROUP_MAX_CUSTOM_LOSSLESS_PACKET_LENGTH if the
+     * packet was lossless.
      */
     TOX_ERR_GROUP_SEND_CUSTOM_PACKET_TOO_LONG,
 
@@ -4541,7 +4553,9 @@ typedef enum Tox_Err_Group_Send_Custom_Private_Packet {
     TOX_ERR_GROUP_SEND_CUSTOM_PRIVATE_PACKET_GROUP_NOT_FOUND,
 
     /**
-     * Message length exceeded TOX_MAX_CUSTOM_PACKET_SIZE.
+     * Message length exceeded TOX_GROUP_MAX_CUSTOM_LOSSY_PACKET_LENGTH if the
+     * packet was lossy, or TOX_GROUP_MAX_CUSTOM_LOSSLESS_PACKET_LENGTH if the
+     * packet was lossless.
      */
     TOX_ERR_GROUP_SEND_CUSTOM_PRIVATE_PACKET_TOO_LONG,