implement recv_buffer size limit and input validation

* Implement recv_buffer size limit and input validation

* Add readLengthPrefixedData helper and refactor some read flows

* Change error to be more accurate

* Add newline to error message

* fix long chat messages kicking clients

* style nitpicks

---------

Co-authored-by: p2r3 <p2r3@p2r3.com>
Co-authored-by: p2r3 <41925384+p2r3@users.noreply.github.com>

include/globals.h

@@ -102,6 +102,9 @@
 // clients from Keep Alive packets.
 #define NETWORK_TIMEOUT_TIME 15000000
 
+// Size of the receive buffer for incoming string data
+#define MAX_RECV_BUF_LEN 256 
+
 // If defined, sends the server brand to clients. Doesn't do much, but will
 // show up in the top-left of the F3/debug menu, in the Minecraft client.
 // You can change the brand string in the "brand" variable in src/globals.c

include/tools.h

@@ -32,7 +32,9 @@ int64_t readInt64 (int client_fd);
 float readFloat (int client_fd);
 double readDouble (int client_fd);
 
+ssize_t readLengthPrefixedData (int client_fd);
 void readString (int client_fd);
+void readStringN (int client_fd, uint32_t max_length);
 
 uint32_t fast_rand ();
 uint64_t splitmix64 (uint64_t state);

src/globals.c

@@ -29,7 +29,7 @@
 #endif
 
 ssize_t recv_count;
-uint8_t recv_buffer[256] = {0};
+uint8_t recv_buffer[MAX_RECV_BUF_LEN] = {0};
 
 uint32_t world_seed = INITIAL_WORLD_SEED;
 uint32_t rng_seed = INITIAL_RNG_SEED;

src/packets.c

@@ -658,10 +658,8 @@ int cs_clickContainer (int client_fd) {
     count = (uint8_t)readVarInt(client_fd);
 
     // ignore components
-    tmp = readVarInt(client_fd);
-    recv_all(client_fd, recv_buffer, tmp, false);
-    tmp = readVarInt(client_fd);
-    recv_all(client_fd, recv_buffer, tmp, false);
+    readLengthPrefixedData(client_fd);
+    readLengthPrefixedData(client_fd);
 
     if (count > 0 && apply_changes) {
       *p_item = item;
@@ -691,10 +689,8 @@ int cs_clickContainer (int client_fd) {
     player->flagval_16 = readVarInt(client_fd);
     player->flagval_8 = readVarInt(client_fd);
     // ignore components
-    tmp = readVarInt(client_fd);
-    recv_all(client_fd, recv_buffer, tmp, false);
-    tmp = readVarInt(client_fd);
-    recv_all(client_fd, recv_buffer, tmp, false);
+    readLengthPrefixedData(client_fd);
+    readLengthPrefixedData(client_fd);
   } else {
     player->flagval_16 = 0;
     player->flagval_8 = 0;
@@ -1117,7 +1113,8 @@ int sc_systemChat (int client_fd, char* message, uint16_t len) {
 // C->S Chat Message
 int cs_chat (int client_fd) {
 
-  readString(client_fd);
+  // To be safe, cap messages to 32 bytes before the buffer length
+  readStringN(client_fd, 224);
 
   PlayerData *player;
   if (getPlayerData(client_fd, &player)) return 1;
@@ -1125,12 +1122,6 @@ int cs_chat (int client_fd) {
   size_t message_len = strlen((char *)recv_buffer);
   uint8_t name_len = strlen(player->name);
 
-  // To be safe, cap messages to 32 bytes before the buffer length
-  if (message_len > 224) {
-    recv_buffer[224] = '\0';
-    message_len = 224;
-  }
-
   if (recv_buffer[0] != '!') { // Standard chat message
 
     // Shift message contents forward to make space for player name tag

src/tools.c

@@ -217,12 +217,45 @@ double readDouble (int client_fd) {
   return output;
 }
 
+// Receive length prefixed data with bounds checking
+ssize_t readLengthPrefixedData (int client_fd) {
+  uint32_t length = readVarInt(client_fd);
+  if (length >= MAX_RECV_BUF_LEN) {
+    printf("ERROR: Received length (%u) exceeds maximum (%u)\n", length, MAX_RECV_BUF_LEN);
+    disconnectClient(&client_fd, -1);
+    recv_count = 0;
+    return 0;
+  }
+  return recv_all(client_fd, recv_buffer, length, false);
+}
+
 // Reads a networked string into recv_buffer
 void readString (int client_fd) {
-  uint32_t length = readVarInt(client_fd);
-  recv_count = recv_all(client_fd, recv_buffer, length, false);
+  recv_count = readLengthPrefixedData(client_fd);
   recv_buffer[recv_count] = '\0';
 }
+// Reads a networked string of up to N bytes into recv_buffer
+void readStringN (int client_fd, uint32_t max_length) {
+  // Forward to readString if max length is invalid
+  if (max_length >= MAX_RECV_BUF_LEN) {
+    readString(client_fd);
+    return;
+  }
+  // Attempt to read full string within maximum
+  uint32_t length = readVarInt(client_fd);
+  if (max_length > length) {
+    recv_count = recv_all(client_fd, recv_buffer, length, false);
+    recv_buffer[recv_count] = '\0';
+    return;
+  }
+  // Read string up to maximum, dump the rest
+  recv_count = recv_all(client_fd, recv_buffer, max_length, false);
+  recv_buffer[recv_count] = '\0';
+  uint8_t dummy;
+  for (uint32_t i = max_length; i < length; i ++) {
+    recv_all(client_fd, &dummy, 1, false);
+  }
+}
 
 uint32_t fast_rand () {
   rng_seed ^= rng_seed << 13;