Repository navigation
Conversation
This branch has not been deployed
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.
Validate reply-path length in repeater anonymous request handlers
MyMesh::onAnonDataRecv() passed &data[5] into handleAnonRegionsReq(), handleAnonOwnerReq() and handleAnonClockReq() without the remaining payload length. Those handlers checked reply_path_len against the 64-byte protocol maximum with Packet::isValidPathLen(), but not against the bytes actually received, before copying the path with Packet::writePath() and using it as the route for the reply.
This is reachable without authentication: any node in radio range can send a direct ANON_REQ using its own keypair. A single-block request claiming a 64-byte reply path makes the repeater copy up to 53 stale stack bytes into the reply path, which is sent unencrypted in the packet header. That stack region is shared with other packet handling and can hold recent data, for example the text of a remote admin CLI command such as a password change. I can share more details privately if useful.
This change adds a defensive len < 5 check in onAnonDataRecv(), passes len - 5 to the three handlers, and has a small extractReplyPath() helper verify that the claimed path bytes are present before updating reply_path_len and copying the path.
Found during an AI-assisted bounds review and checked by hand against the ANON_REQ layout. Tested with an AddressSanitizer harness covering valid, boundary and truncated reply paths, the native unit tests (pio test), and RAK_4631_repeater and Heltec_v3_repeater firmware builds. Not tested on hardware.