Current Behavior
INAV accepts a NAV_WP_ACTION_JUMP waypoint whose target field p1 is 0.
JUMP targets are represented as 1-based waypoint numbers when uploaded, and setWaypoint() converts them to a zero-based index:
if (wpData->action == NAV_WP_ACTION_JUMP) {
posControl.waypointList[wpNumber - 1].p1 -= 1;
}
Therefore, a JUMP waypoint uploaded with p1 == 0 is stored with p1 == -1.
There is no lower-bound validation before this conversion.
This can subsequently result in an out-of-bounds access to posControl.waypointList.
In particular, the JUMP waypoint validation code checks whether the target exceeds waypointCount, but does not check whether it is negative:
if (wp == posControl.startWpIndex ||
posControl.waypointList[wp].p1 >= posControl.waypointCount ||
...
) {
return NAV_ARMING_BLOCKER_JUMP_WAYPOINT_ERROR;
}
uint16_t target =
posControl.waypointList[wp].p1 + posControl.startWpIndex;
For a normal mission with startWpIndex == 0, the stored value p1 == -1 is converted to uint16_t, producing 65535. The subsequent access to:
posControl.waypointList[target]
therefore reads far beyond the waypoint table.
If the malformed JUMP reaches mission execution, the navigation FSM can also assign the negative target to activeWaypointIndex, resulting in an access equivalent to:
posControl.waypointList[-1]
The trigger does not require a malformed MSP frame. A structurally valid MSP_SET_WP request containing a semantically invalid JUMP target (p1 == 0) is sufficient.
Steps to Reproduce
-
Build and run the current INAV master/HEAD revision on a supported target or suitable test environment.
-
Upload a waypoint mission using MSP_SET_WP containing a JUMP waypoint with:
action = NAV_WP_ACTION_JUMP
p1 = 0
The rest of the MSP_SET_WP message can be structurally valid.
- Observe that
setWaypoint() accepts the waypoint and executes:
posControl.waypointList[wpNumber - 1].p1 -= 1;
causing the stored JUMP target to become -1.
- Trigger waypoint mission validation, for example as part of the arming checks. The negative target is not rejected before it is used to calculate:
uint16_t target =
posControl.waypointList[wp].p1 + posControl.startWpIndex;
For startWpIndex == 0, this produces target == 65535, and the subsequent posControl.waypointList[target] access is out of bounds.
The issue can also propagate into waypoint execution if the malformed mission reaches the navigation state machine, where the negative target may cause waypointList[-1] to be accessed.
Expected behavior
INAV should reject a JUMP waypoint whose target is outside the valid waypoint-number range before converting the 1-based target into a zero-based array index.
In particular, p1 == 0 should be rejected as an invalid JUMP target and should never be stored as -1 or subsequently used to index waypointList.
Mission validation should also independently verify both the lower and upper bounds of every JUMP target before accessing the waypoint table.
Suggested solution(s)
Validate the JUMP target before subtracting one in setWaypoint(). For example, require the supplied 1-based p1 value to be at least 1 and within the valid waypoint range before converting it to a zero-based index.
Additionally, the JUMP validation code should explicitly reject negative stored targets before calculating or dereferencing the target waypoint, e.g. conceptually:
if (posControl.waypointList[wp].p1 < 0 ||
posControl.waypointList[wp].p1 >= posControl.waypointCount) {
return NAV_ARMING_BLOCKER_JUMP_WAYPOINT_ERROR;
}
Adding bounds checks at both mission ingestion and mission execution/validation would provide defense in depth and prevent invalid mission data from becoming an unsafe array index.
Additional context
The affected waypoint table is part of the global navigation state (posControl), so the resulting access is outside the bounds of the statically allocated waypoint array.
The immediately demonstrated memory-safety primitive is an out-of-bounds read. Depending on how invalid waypoint data propagates through later navigation states, additional unsafe accesses may also be possible.
This issue is not specific to malformed MSP framing. It can be triggered using an otherwise valid MSP_SET_WP message containing the invalid semantic combination:
NAV_WP_ACTION_JUMP
p1 = 0
- FC Board name and vendor: Not board-specific; reproduced from the common navigation/MSP code path
- INAV version string: Current
master / HEAD revision
Current Behavior
INAV accepts a
NAV_WP_ACTION_JUMPwaypoint whose target fieldp1is0.JUMP targets are represented as 1-based waypoint numbers when uploaded, and
setWaypoint()converts them to a zero-based index:Therefore, a JUMP waypoint uploaded with
p1 == 0is stored withp1 == -1.There is no lower-bound validation before this conversion.
This can subsequently result in an out-of-bounds access to
posControl.waypointList.In particular, the JUMP waypoint validation code checks whether the target exceeds
waypointCount, but does not check whether it is negative:For a normal mission with
startWpIndex == 0, the stored valuep1 == -1is converted touint16_t, producing65535. The subsequent access to:therefore reads far beyond the waypoint table.
If the malformed JUMP reaches mission execution, the navigation FSM can also assign the negative target to
activeWaypointIndex, resulting in an access equivalent to:The trigger does not require a malformed MSP frame. A structurally valid
MSP_SET_WPrequest containing a semantically invalid JUMP target (p1 == 0) is sufficient.Steps to Reproduce
Build and run the current INAV
master/HEAD revision on a supported target or suitable test environment.Upload a waypoint mission using
MSP_SET_WPcontaining a JUMP waypoint with:The rest of the
MSP_SET_WPmessage can be structurally valid.setWaypoint()accepts the waypoint and executes:causing the stored JUMP target to become
-1.For
startWpIndex == 0, this producestarget == 65535, and the subsequentposControl.waypointList[target]access is out of bounds.The issue can also propagate into waypoint execution if the malformed mission reaches the navigation state machine, where the negative target may cause
waypointList[-1]to be accessed.Expected behavior
INAV should reject a JUMP waypoint whose target is outside the valid waypoint-number range before converting the 1-based target into a zero-based array index.
In particular,
p1 == 0should be rejected as an invalid JUMP target and should never be stored as-1or subsequently used to indexwaypointList.Mission validation should also independently verify both the lower and upper bounds of every JUMP target before accessing the waypoint table.
Suggested solution(s)
Validate the JUMP target before subtracting one in
setWaypoint(). For example, require the supplied 1-basedp1value to be at least1and within the valid waypoint range before converting it to a zero-based index.Additionally, the JUMP validation code should explicitly reject negative stored targets before calculating or dereferencing the target waypoint, e.g. conceptually:
Adding bounds checks at both mission ingestion and mission execution/validation would provide defense in depth and prevent invalid mission data from becoming an unsafe array index.
Additional context
The affected waypoint table is part of the global navigation state (
posControl), so the resulting access is outside the bounds of the statically allocated waypoint array.The immediately demonstrated memory-safety primitive is an out-of-bounds read. Depending on how invalid waypoint data propagates through later navigation states, additional unsafe accesses may also be possible.
This issue is not specific to malformed MSP framing. It can be triggered using an otherwise valid
MSP_SET_WPmessage containing the invalid semantic combination:master/ HEAD revision