🛡️ Sentinel: [HIGH] Buffer Overflow in Network Encoding due to missing bounds check

[igorls]

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, 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.

Fix:
Added buf.len checks to encodePing, encodeAck, and encodePingReq before writing into the slice.

Verification:

• Built and ran tests using zig build test and zig build -Doptimize=ReleaseSafe. All tests pass successfully.

---

PR created automatically by Jules for task 10146114884272453900 started by @igorls

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[Copilot]

Pull request overview

This PR fixes a potential buffer overflow in src/protocol/codec.zig by adding bounds checks to encodePing, encodeAck, and encodePingReq. Without these checks, if the caller supplied a buffer smaller than the encoded message size, the encoder could write out-of-bounds memory. The PR also documents the vulnerability in the project's security log (.jules/sentinel.md).

Changes:

• Added buf.len < required pre-flight checks to encodePing, encodeAck, and encodePingReq, returning error.BufferTooShort if the buffer is insufficient.
• Documented the vulnerability and the fix in .jules/sentinel.md.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/protocol/codec.zig	Adds bounds checks (3 new guard blocks) to the three SWIM message encoders
.jules/sentinel.md	Appends a new dated security log entry describing the vulnerability, cause, and fix

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The required size calculation for encodePing uses ping.gossip.len * GOSSIP_ENTRY_SIZE, but the actual encoding loop at line 38 only writes @min(ping.gossip.len, 255) entries (gossip_count). If ping.gossip.len > 255, the bounds check will compute a required value that is much larger than what will actually be written, causing a spurious error.BufferTooShort even when the buffer is sufficient to hold all 255 gossip entries (up to ~22,755 bytes). In swim.zig, this error is silently swallowed with catch return, meaning pings would be silently dropped when ping.gossip.len > 255.

The required calculation should use @min(ping.gossip.len, 255) * GOSSIP_ENTRY_SIZE to correctly reflect how many bytes will actually be written.

[Copilot]

Same issue as in encodePing: the required size uses ack.gossip.len * GOSSIP_ENTRY_SIZE, but the encoder only writes @min(ack.gossip.len, 255) entries. If ack.gossip.len > 255, the check returns error.BufferTooShort even though the buffer may be large enough for all 255 entries. The required calculation should use @min(ack.gossip.len, 255) * GOSSIP_ENTRY_SIZE to match the actual bytes written.