🛡️ Sentinel: [CRITICAL] Fix buffer overflows in EAD protocol handlers

.jules/sentinel.md

@@ -18,3 +18,7 @@
 **Vulnerability:** The `ead` service writes its process ID to a file using `sprintf(pid, "%d\n", getpid())` into a stack-allocated buffer `char pid[8];`. Since `getpid()` can return up to 7 digits on modern Linux systems, adding `\n` and `\0` requires at least 9 bytes, overflowing the 8-byte buffer.
 **Learning:** Hardcoded small buffer sizes for formatting integer PIDs can lead to stack memory corruption (CWE-121) as OS configurations like `kernel.pid_max` allow PIDs to exceed traditional limits.
 **Prevention:** Always allocate sufficiently large stack buffers (e.g., 32 bytes) when formatting PIDs, or preferably use `snprintf` with `sizeof(buffer)` to enforce explicit bounds checking.
+## 2024-05-23 - Bounds checks in network payload handlers
+**Vulnerability:** EAD service and client C programs had multiple instances where data derived from raw network UDP packets was extracted and processed without adequate bounds checking against the receiving buffer's actual memory size. The payload length (`msg->len`) derived from `ntohl` could potentially underflow into a negative value, or specify a valid size that was still larger than the underlying allocated stack/BSS buffer, leading to out-of-bounds writes via `memcpy` or array assignment.
+**Learning:** Legacy C network services often perform implicit trust checks on received lengths before utilizing them. Because `memcpy` takes a `size_t` (an unsigned integer), a negative signed `len` calculation effectively wraps around to a very large positive number. This highlights the importance of checking `len <= 0` simultaneously with `len >= MAX_BUFFER_SIZE`.
+**Prevention:** Always validate extracted lengths from network packets explicitly. Ensure lengths are mathematically bound to be greater than zero, and strictly less than the remaining unread capacity of the static/dynamic memory buffer allocated to store the packet data. Utilize `sizeof()` safely to determine max boundaries dynamically.

package/network/services/ead/src/ead-client.c

@@ -151,7 +151,7 @@ handle_pong(void)
 	struct ead_msg_pong *pong = EAD_DATA(msg, pong);
 	int len = ntohl(msg->len) - sizeof(struct ead_msg_pong);
 
-	if (len <= 0)
+	if (len <= 0 || len >= sizeof(msgbuf) - sizeof(struct ead_msg) - sizeof(struct ead_msg_pong))
 		return false;
 
 	pong->name[len] = 0;
@@ -167,6 +167,9 @@ handle_prime(void)
 {
 	struct ead_msg_salt *sb = EAD_DATA(msg, salt);
 
+	if (sb->len > MAXSALTLEN)
+		return false;
+
 	salt.len = sb->len;
 	memcpy(salt.data, sb->salt, salt.len);
 
@@ -191,6 +194,9 @@ handle_b(void)
 	struct ead_msg_number *num = EAD_DATA(msg, number);
 	int len = ntohl(msg->len) - sizeof(struct ead_msg_number);
 
+	if (len <= 0 || len > MAXPARAMLEN)
+		return false;
+
 	B.data = bbuf;
 	B.len = len;
 	memcpy(bbuf, num->data, len);
@@ -220,7 +226,7 @@ handle_cmd_data(void)
 	struct ead_msg_cmd_data *cmd = EAD_ENC_DATA(msg, cmd_data);
 	int datalen = ead_decrypt_message(msg) - sizeof(struct ead_msg_cmd_data);
 
-	if (datalen < 0)
+	if (datalen < 0 || datalen > sizeof(msgbuf) - sizeof(struct ead_msg) - sizeof(struct ead_msg_encrypted) - sizeof(struct ead_msg_cmd_data))
 		return false;
 
 	if (datalen > 0) {

package/network/services/ead/src/ead.c

@@ -430,7 +430,7 @@ handle_send_a(struct ead_packet *pkt, int len, int *nstate)
 	struct ead_msg_number *number = EAD_DATA(msg, number);
 	len = ntohl(msg->len) - sizeof(struct ead_msg_number);
 
-	if (len > MAXPARAMLEN + 1)
+	if (len <= 0 || len > MAXPARAMLEN + 1)
 		return false;
 
 	A.len = len;
@@ -487,7 +487,7 @@ handle_send_cmd(struct ead_packet *pkt, int len, int *nstate)
 	datalen = ead_decrypt_message(msg) - sizeof(struct ead_msg_cmd);
 	if (datalen <= 0)
 		return false;
-	if (datalen > 1024)
+	if (datalen > 1024 || datalen >= sizeof(pktbuf_b) - sizeof(struct ead_msg) - sizeof(struct ead_msg_encrypted) - sizeof(struct ead_msg_cmd))
 		return false;
 
 	type = ntohs(cmd->type);