-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Verify frame length when handling CRSF_FRAMETYPE_MSP_REQ/CRSF_FRAMETYPE_MSP_WRITE #11210
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: maintenance-9.x
Are you sure you want to change the base?
Verify frame length when handling CRSF_FRAMETYPE_MSP_REQ/CRSF_FRAMETYPE_MSP_WRITE #11210
Conversation
…AMETYPE_MSP_WRITE
Branch Targeting SuggestionYou've targeted the
If This is an automated suggestion to help route contributions to the appropriate branch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds frame length validation for CRSF MSP frame types to prevent potential underflow issues when processing frames. It addresses issue #11209 by ensuring that MSP request and write frames have the minimum required length before processing.
Changes:
- Added validation to check that
frameLength >= 4before processing CRSF_FRAMETYPE_MSP_REQ and CRSF_FRAMETYPE_MSP_WRITE frames - Added error handling to discard frames that fail validation by setting
crsfFrameDone = false
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (crsfFrame.frame.frameLength >= 4) { | ||
| uint8_t *frameStart = (uint8_t *)&crsfFrame.frame.payload + CRSF_FRAME_ORIGIN_DEST_SIZE; | ||
| if (bufferCrsfMspFrame(frameStart, crsfFrame.frame.frameLength - 4)) { |
Copilot
AI
Jan 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The magic number 4 should be replaced with the named constant CRSF_FRAME_LENGTH_EXT_TYPE_CRC for better code maintainability and clarity. This constant represents the overhead for extended frames (Type + Origin + Destination + CRC).
| if (crsfFrame.frame.frameLength >= 4) { | |
| uint8_t *frameStart = (uint8_t *)&crsfFrame.frame.payload + CRSF_FRAME_ORIGIN_DEST_SIZE; | |
| if (bufferCrsfMspFrame(frameStart, crsfFrame.frame.frameLength - 4)) { | |
| if (crsfFrame.frame.frameLength >= CRSF_FRAME_LENGTH_EXT_TYPE_CRC) { | |
| uint8_t *frameStart = (uint8_t *)&crsfFrame.frame.payload + CRSF_FRAME_ORIGIN_DEST_SIZE; | |
| if (bufferCrsfMspFrame(frameStart, crsfFrame.frame.frameLength - CRSF_FRAME_LENGTH_EXT_TYPE_CRC)) { |
|
Thanks for this. Has this been observed actually happening? I asked because I am deciding whether to merge it today for INAV 9.0.0, or for INAV 9.1 in March-April. |
This PR addresses #11209