Skip to content

Out-of-bounds access from invalid NAV_WP_ACTION_JUMP target #11841

Description

@Arslan8

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

  1. Build and run the current INAV master/HEAD revision on a supported target or suitable test environment.

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

  1. Observe that setWaypoint() accepts the waypoint and executes:
posControl.waypointList[wpNumber - 1].p1 -= 1;

causing the stored JUMP target to become -1.

  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

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions