Skip to content

Conversation

@VoodooChild99
Copy link

This PR addresses #11209

@github-actions
Copy link

Branch Targeting Suggestion

You've targeted the master branch with this PR. Please consider if a version branch might be more appropriate:

  • maintenance-9.x - If your change is backward-compatible and won't create compatibility issues between INAV firmware and Configurator 9.x versions. This will allow your PR to be included in the next 9.x release.

  • maintenance-10.x - If your change introduces compatibility requirements between firmware and configurator that would break 9.x compatibility. This is for PRs which will be included in INAV 10.x

If master is the correct target for this change, no action is needed.


This is an automated suggestion to help route contributions to the appropriate branch.

@sensei-hacker sensei-hacker changed the base branch from master to maintenance-9.x December 28, 2025 21:42
@sensei-hacker sensei-hacker added this to the 9.1 milestone Dec 28, 2025
@sensei-hacker sensei-hacker requested a review from Copilot January 10, 2026 22:08
Copy link
Contributor

Copilot AI left a 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 >= 4 before 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.

Comment on lines +176 to +178
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)) {
Copy link

Copilot AI Jan 10, 2026

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).

Suggested change
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)) {

Copilot uses AI. Check for mistakes.
@sensei-hacker
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants