diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 7a55c59..ba35173 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -16,3 +16,8 @@ **Vulnerability:** Intermediate key material (`secret` extracted from chaining key, and expanded `t*_plus` buffers) in `src/wireguard/crypto.zig`'s HKDF implementation (`kdf1`, `kdf2`, `kdf3`) was left on the stack without being zeroed. These buffers contain sensitive pseudorandom keys derived from the handshake chaining key. **Learning:** Helper functions often create temporary buffers on the stack which persist until overwritten. Unlike the main handshake state struct, these are ephemeral but critical. Zig does not automatically zero stack variables on return. **Prevention:** Always use `defer std.crypto.secureZero(u8, &var);` immediately after declaring any variable that will hold sensitive key material, especially in crypto primitives. + +## 2026-10-24 - [HIGH] Buffer Overflow in Network Encoding due to missing bounds check +**Vulnerability:** In `src/protocol/codec.zig`, functions like `encodePing` and `encodeAck` did not verify if the output buffer (`buf`) was large enough to hold the encoded protocol message, including its dynamic number of gossip entries. The encoder could potentially write out-of-bounds if given a small buffer. Although `encodePing` is often passed a generously sized 1500-byte stack buffer (e.g. in `swim.zig`'s `broadcastLeave`), a large number of gossip entries (e.g., up to the maximum 255) could exceed this limit. 1500 bytes allows roughly 16 gossip entries ($16 \times 89 = 1424$), but the encoder was capping the number at 255, permitting up to ~22,000 bytes. +**Learning:** Even internal encoding functions that serialize structurally sound structs must perform bounds checking on the output slice, especially when elements are variable in length or quantity. The max count limit applied was logically sound for the protocol struct, but completely unbounded relative to the target byte slice length. +**Prevention:** Always use explicit bounds checking (`if (buf.len < required) return error.BufferTooShort;`) at the beginning of any serialization logic. Never assume the caller has provided an adequately sized buffer for unbounded or large slices. diff --git a/src/protocol/codec.zig b/src/protocol/codec.zig index 4018f49..112e1c9 100644 --- a/src/protocol/codec.zig +++ b/src/protocol/codec.zig @@ -17,6 +17,9 @@ const GOSSIP_ENTRY_SIZE = 32 + 1 + 8 + 1 + 4 + 2 + 1 + 32 + 1 + 4 + 2 + 1; // 89 /// Encode a Ping message into a buffer. Returns bytes written. pub fn encodePing(buf: []u8, ping: messages.Ping) !usize { + const required = 1 + 32 + 8 + 1 + ping.gossip.len * GOSSIP_ENTRY_SIZE; + if (buf.len < required) return error.BufferTooShort; + var pos: usize = 0; // Type tag @@ -45,6 +48,9 @@ pub fn encodePing(buf: []u8, ping: messages.Ping) !usize { /// Encode an Ack message into a buffer. Returns bytes written. pub fn encodeAck(buf: []u8, ack: messages.Ack) !usize { + const required = 1 + 32 + 8 + 1 + ack.gossip.len * GOSSIP_ENTRY_SIZE; + if (buf.len < required) return error.BufferTooShort; + var pos: usize = 0; buf[pos] = @intFromEnum(messages.MessageType.ack); @@ -69,6 +75,9 @@ pub fn encodeAck(buf: []u8, ack: messages.Ack) !usize { /// Encode a PingReq message. pub fn encodePingReq(buf: []u8, req: messages.PingReq) !usize { + const required = 1 + 32 + 32 + 8; + if (buf.len < required) return error.BufferTooShort; + var pos: usize = 0; buf[pos] = @intFromEnum(messages.MessageType.ping_req);