From b8aa21cc447e866633fb5a2d26b0f86b3268055e Mon Sep 17 00:00:00 2001 From: jfreegman Date: Thu, 24 Nov 2022 11:41:26 -0500 Subject: [PATCH] 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. --- .../docker/tox-bootstrapd.sha256 | 2 +- toxcore/group_chats.c | 51 ++++++++++++++----- toxcore/group_common.h | 16 +++--- toxcore/tox.c | 4 +- toxcore/tox.h | 18 ++++++- 5 files changed, 68 insertions(+), 23 deletions(-) diff --git a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 index 52982fe9..035b4d94 100644 --- a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 +++ b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 @@ -1 +1 @@ -769233afaac07be03c094411e6ec8f031bde41beae475c74e154e51e51e9168b /usr/local/bin/tox-bootstrapd +cbd8bea9d23a961f27aacd35c4509fc1c22d72356f61d1ec74cc469b6b14490d /usr/local/bin/tox-bootstrapd diff --git a/toxcore/group_chats.c b/toxcore/group_chats.c index 9901b801..e502a722 100644 --- a/toxcore/group_chats.c +++ b/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; } diff --git a/toxcore/group_common.h b/toxcore/group_common.h index 8ee07955..d350b464 100644 --- a/toxcore/group_common.h +++ b/toxcore/group_common.h @@ -23,9 +23,16 @@ #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 @@ -33,9 +40,6 @@ #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) diff --git a/toxcore/tox.c b/toxcore/tox.c index 852f5600..6f2261fd 100644 --- a/toxcore/tox.c +++ b/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; diff --git a/toxcore/tox.h b/toxcore/tox.h index 125de83b..06146c60 100644 --- a/toxcore/tox.h +++ b/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,