Validate decrypted PATH path_len against MAX_PATH_SIZE#1662
Open
weebl2000 wants to merge 1 commit intomeshcore-dev:devfrom
Open
Validate decrypted PATH path_len against MAX_PATH_SIZE#1662weebl2000 wants to merge 1 commit intomeshcore-dev:devfrom
weebl2000 wants to merge 1 commit intomeshcore-dev:devfrom
Conversation
5e59b41 to
914b024
Compare
3 tasks
d46fd6f to
dd5c0a0
Compare
dd5c0a0 to
82573b6
Compare
The path_len field inside decrypted PATH payloads was validated against the decrypted buffer size but not against MAX_PATH_SIZE (64). A malicious contact could send a PATH packet with path_len up to 178, overflowing out_path[64] in onPeerPathRecv and packet->path[64] in sendDirect. Add a MAX_PATH_SIZE check after parsing path_len from the decrypted PATH payload. Also add defensive bounds checks in sendDirect for both the TRACE payload-append path and the normal path-copy path.
82573b6 to
33868dc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Severity: High
Summary
The
path_lenfield inside decrypted PATH payloads is validated against the decrypted buffer size (by #1654) but not againstMAX_PATH_SIZE(64). A malicious contact can craft a PATH packet where the innerpath_lenencodes up to 189 bytes of path data (e.g.path_len = 0xBF: hash_size=3, hash_count=63). This passes #1654's payload-length check (the decrypted buffer can hold ~176 bytes) but far exceeds the 64-byteout_path[]andpath[]buffers used downstream.Actual impact (traced through full code flow)
The
out_path[64]struct overflow described originally is mitigated by an existing check inPacket::writePath(), which returns 0 without writing ifhash_count * hash_size > MAX_PATH_SIZE. However,Packet::copyPath()ignores that return value and always returns the rawpath_len, so:Corrupt
out_path_lenmetadata — allonPeerPathRecvimplementations store the return value ofcopyPathintoclient->out_path_len/from.out_path_len. WhenwritePathsilently rejects the oversized path,out_path_lenis set to the attacker's value (e.g. 189) whileout_path[]contains stale data. Subsequent sends to this contact use the corruptout_path_len, producing malformed packets on the wire (path_len header claims many hashes, no actual path data follows).sendDirectTRACE branch: real buffer overflow — the TRACE branch does an unboundedmemcpy(&packet->payload[packet->payload_len], path, path_len)without checkingpayload_len + path_len <= sizeof(packet->payload). While not directly reachable from the PATH receive handler,sendDirectis called from many places and this is a genuine overflow ofpayload[184].sendDirectnon-TRACE branch: malformed packet —copyPathcatches the write viawritePathbut stores the boguspath_leninpacket->path_len, producing a corrupt packet.Who can exploit this: any peer in your contacts list (they need a shared key for the MAC/decryption to pass).
What it takes: a single crafted PATH packet over RF.
What users might see
A malicious contact could corrupt a node's routing state for the affected contact (stale/garbage path used for all subsequent sends), cause malformed packets to propagate through the mesh, or potentially crash the node via the TRACE
sendDirectoverflow path.Fix
hash_size * hash_count > MAX_PATH_SIZEcheck inonRecvPacketafter parsingpath_lenfrom the decrypted PATH payload, rejecting the packet before it flows intoonPeerPathRecvorsendDirectsendDirectTRACE branch:payload_len + path_len > sizeof(packet->payload)→ free packet and returnpath_len > MAX_PATH_SIZEguard insendDirectnon-TRACE branch as defense-in-depthNote:
copyPathreturn value bugPacket::copyPath()always returns the rawpath_leneven whenwritePath()rejected the write. This PR's early-reject inonRecvPacketprevents exploitation via the PATH vector, but thecopyPathAPI remains a footgun for future callers. A follow-up fix would be:Test plan
Heltec_v3_companion_radio_bleBuild firmware: Build from this branch