Bound LCM protocol payloads#90
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens the LCM (Light Control Module) protocol handling by bounding/normalizing incoming fields before storing them and before forwarding them back out, preventing out-of-bounds copies and invalid/stale payload behavior.
Changes:
- Bound and null-terminate copied LCM name strings to avoid overruns.
- Clamp brightness bytes to the protocol’s 0–100 range and cap stored/forwarded payload length to fixed capacity.
- Reset per-request payload state and simplify the enabled-only light-control handler.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lukash
left a comment
There was a problem hiding this comment.
I think the changes in the two commits in this PR should ideally be split into 3 commits:
- the simplification of the name copying code
- limiting the brightnesses to max 100
- handling the payload size overflow / clearing on short request
d9bf0b3 to
dff2465
Compare
lukash
left a comment
There was a problem hiding this comment.
A couple more remarks...
Also, applies to all the PRs, please don't use the conventional commit messages (prefixed with fix: ). I prefer component prefixes is applicable. And after the prefix, please start with a capital letter just like you do if there's no prefix (look at other commits messages if unclear). I won't force you to rephrase the current commit messages if you don't want to (just let me know so I don't wait).
dff2465 to
52419ea
Compare
52419ea to
cc193b2
Compare
Bounds LCM protocol inputs before storing or forwarding them.
Changes
Behavior
Valid LCM traffic is unchanged. Oversized names and payloads stay within their fixed buffers, and a rejected short light-control request cannot leave an older payload queued for the next poll response.
I was able to cause a crash on the existing code with enough BTLE packets.