TRACE handler reads past the received payload when hop size exceeds remaining data

Open Beginner friendly
  • Is something wrong?
#3,781 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 2 days

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
78/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
c
Domain
networking

Research direction

Start in src/Mesh.cpp, in Mesh::onRecvPacket's TRACE branch, and compare the remaining payload length with the hop size before isHashMatch reads the packet. The report includes a reproducing case: 3-byte path mode with flags declaring 4-byte hops and only 3 hash bytes. Check existing TRACE tests or add coverage for that case; done means the receiver does not read beyond payload_len and handles the malformed trace consistently.

Written by the indexing model from the issue text.

Description

Firmware: repeater-v1.18.0 (81dbae5), same code on current main

Description

In Mesh::onRecvPacket (src/Mesh.cpp), the TRACE branch checks:

uint8_t len = pkt->payload_len - i;
uint16_t offset = (uint16_t)pkt->path_len << path_sz;
if (offset >= len) {
  onTraceRecv(...);
} else if (self_id.isHashMatch(&pkt->payload[i + offset], 1 << path_sz) && ...) {

There is no check that len - offset >= (1 << path_sz). If a TRACE's flags declare a larger hop size than the data actually carries, isHashMatch compares bytes beyond payload_len. Those bytes are stale contents of the packet buffer, so the forward decision becomes nondeterministic.

How it shows up

A third-party companion (Wadamesh, T-Deck) in 3-byte path mode sends TRACE with flags = 2 (4-byte hops) and only 3 hash bytes. Against a 1.18 repeater, the first trace was not forwarded; later identical traces (10/10) were. That matches a stale 4th byte that sometimes equals the repeater's pub_key[3]. The sender is wrong too (ALLFATHER-BV/wadamesh#818), but the receiver should not depend on undefined memory.

Suggested fix

uint8_t hsz = 1 << path_sz;
if (offset >= len) {
  onTraceRecv(...);
} else if (len - offset >= hsz &&
           self_id.isHashMatch(&pkt->payload[i + offset], hsz) && ...) {

Optionally treat len % hsz != 0 as malformed and drop the packet.

Dominant language
C
Stars
3.9k
Forks
1.5k
Avg merge
2d 21h
Merged PRs (30d)
34

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from meshcore-dev/MeshCore

All issues in meshcore-dev/MeshCore

Similar issues

More C issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.