Skip to content

fix: bounds-check input length in Packet::readFrom() - #3552

Open
kim7tin wants to merge 1 commit into
meshcore-dev:devfrom
kim7tin:fix/packet-readfrom-bounds
Open

kim7tin wants to merge 1 commit into
meshcore-dev:devfrom
kim7tin:fix/packet-readfrom-bounds

Conversation

@kim7tin

@kim7tin kim7tin commented Oct 5, 2026

Copy link
Copy Markdown

Problem

Packet::readFrom() reads the header, the 4 transport-code bytes, path_len and up to 64 path bytes
before it looks at len. The only length check (i >= len) ran after those reads, so a truncated
packet made it read up to ~70 bytes past the end of src. The packet was still rejected afterwards,
but the out-of-bounds read had already happened.

There is no out-of-bounds write: the path/payload copies are already bounded by isValidPathLen()
and sizeof(payload). On the radio path the receive buffer is large, so this is mostly benign there;
it matters for callers that pass a tightly sized buffer (bridges, network/serial transports, host builds).

Change

Check len before each read:

  • len >= 2 (header + path_len)
  • len >= 6 when the route type has transport codes
  • i + path_byte_len < len before copying the path (also guarantees at least one payload byte,
    replacing the old i >= len check)

No change to the packet format or to the result for valid packets.

Tests

New host test test/test_packet (pio test -e native -f test_packet): empty/header-only input,
truncated transport codes, path longer than input, path with no payload, reserved path_len,
valid flood and transport packets, and a writeTo()/readFrom() round trip. Inputs use exactly-sized
heap buffers so a sanitizer build flags any read past len.
Full pio test -e native: 54 passed, 1 suite skipped (test_companion_node_prefs, unrelated).

readFrom() read the header, transport codes, path_len and up to 64 path
bytes before looking at 'len', so a truncated packet made it read past
the end of the source buffer (the packet was rejected afterwards, but the
over-read had already happened). Check 'len' before each read.

Add host tests (test_packet) for truncated header, transport codes and
path, invalid path_len, and valid round trips.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@neilalexander

Copy link
Copy Markdown
Contributor

See also #3521, which covers this case and various others.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants