Reject JUMP waypoints with an out-of-range target by Raffi1202 · Pull Request #11900 · iNavFlight/inav

Raffi1202 · 2026-09-10T17:53:53Z

Problem

Fixes #11841. The reporter uploaded a mission via MSP_SET_WP containing a JUMP waypoint with p1 = 0. INAV accepted it: setWaypoint() converts the 1-based target to a 0-based index, so the stored target became -1. The arming check then computed uint16_t target = p1 + startWpIndex = 65535 and read posControl.waypointList[65535]; if the mission ran, the FSM assigned the negative value to activeWaypointIndex. @breadoven confirmed the bug.

Cause

src/main/navigation/navigation.c:5482 (maintenance-10.x): posControl.waypointList[wpNumber - 1].p1 -= 1; runs with no lower-bound check on p1. navigationIsBlockingArming() at navigation.c:6369 only tests p1 >= waypointCount, not p1 < 0, before the lookup at :6375. The FSM uses the stored index unchecked at :3050.

Change

In setWaypoint(), a JUMP with p1 < 1 or p1 > NAV_MAX_WAYPOINTS returns before the waypoint is copied and before the index conversion, so nothing is stored. The check runs after the wpNumber == 1 reset, so a rejected first waypoint leaves the list empty (waypointCount 0, waypointListValid false) instead of keeping the previous mission; that reset now also clears the static nonGeoWaypointCount. navigationIsBlockingArming() additionally returns NAV_ARMING_BLOCKER_JUMP_WAYPOINT_ERROR for a negative stored p1, which covers the CLI wp command (cli.c:2015, :2031) that writes p1 straight into waypointList.

Test

Not run on hardware or SITL. Cause verified by reading navigation.c:5482 and :6369 on maintenance-10.x; the MSP_SET_WP handler (fc_msp.c:3336) and the CLI wp path were traced to confirm both entry points are covered. No unit test exercises setWaypoint() or navigationIsBlockingArming() (navigation_unittest.cc.txt is disabled). Built by fork CI: pending. Compiled for all targets and the four SITL builds on the fork, green: https://github.com/Raffi1202/inav/actions/runs/34770668275

Flash / RAM

Builds clean on all targets. No size comparison yet: the fork build has no baseline for this branch, and the upstream size report runs once CI is released for this PR.

Docs

No documentation change needed: docs/Navigation.md:86 (CLI p1 = target WP index) and the MSP_SET_WP reference in docs/development/msp/README.md:2485 describe the field, not the handling of an out-of-range target, which previously had no defined behaviour.

JUMP targets are uploaded as 1 based WP numbers and converted to a zero based index on storage, so a target of 0 ended up stored as -1 and was later used to index the waypoint list. Reject such uploads and refuse a negative target in the mission validation as well. Fixes iNavFlight#11841

qodo-code-review · 2026-09-11T16:02:17Z

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

qodo-free-for-open-source-projects · 2026-09-11T16:03:26Z

PR Summary by Qodo

Reject out-of-range JUMP waypoint targets

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Rejects uploaded JUMP targets outside valid one-based waypoint bounds before index conversion.
• Blocks arming when stored JUMP targets are negative, including CLI-created missions.

Diagram

graph TD
  Upload["Mission Upload"] --> Check{"Target in bounds?"} -->|"Valid; convert index"| Store["Waypoint List"] --> Arm{"Stored target valid?"} -->|Valid| Execute["Mission Execution"]
  Check -->|"Out of range"| Reject["Reject Waypoint"]
  CLI["CLI Waypoint"] --> Store
  Arm -->|"Negative or too high"| Block["Block Arming"]

High-Level Assessment

The following are alternative approaches to this PR:

1. Extract a shared JUMP target validator

➕ Enables focused unit tests for boundary values.
➕ Centralizes JUMP target invariants for future input paths.
➖ Upload and storage use different numbering domains and upper bounds.
➖ Expands a narrowly scoped safety fix into a larger refactor.

Recommendation: Keep the PR's layered validation for this targeted fix: upload validation prevents invalid one-based values from becoming indexes, while arming validation protects direct CLI writes. A shared, representation-aware helper with boundary tests would be a useful follow-up, but is not required to correct the vulnerability.

Files changed (1) +6 / -1

Bug fix (1) +6 / -1

navigation.cValidate JUMP targets before storage and arming +6/-1

Validate JUMP targets before storage and arming

• Rejects uploaded JUMP waypoints whose one-based target falls outside 1 through NAV_MAX_WAYPOINTS before conversion to a zero-based index. Also blocks arming when a stored JUMP target is negative, covering waypoints written directly through the CLI.

src/main/navigation/navigation.c

qodo-free-for-open-source-projects · 2026-09-11T16:05:08Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~Rejected upload keeps old mission active~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

setWaypoint() returns for an invalid first JUMP before resetWaypointList(), preserving the prior
waypoint list, count, and validity flag. When waypoint 1 of an MSP upload or nonvolatile reload has
an out-of-range target, the caller receives no failure and the previously loaded mission remains
executable.

Code

src/main/navigation/navigation.c[R5472-5473]

+            if (wpData->action == NAV_WP_ACTION_JUMP && (wpData->p1 < 1 || wpData->p1 > NAV_MAX_WAYPOINTS)) {
+                return;

Evidence
The added return at lines 5472-5473 precedes the only reset at lines 5480-5482. MSP calls the void
function without checking acceptance, mission activation trusts the unchanged count and validity
flag, and nonvolatile loading can likewise return success based on that stale flag.
src/main/navigation/navigation.c[5471-5483]
src/main/navigation/navigation.c[5506-5522]
src/main/fc/fc_msp.c[3323-3354]
src/main/navigation/navigation.c[2231-2237]
src/main/navigation/navigation.c[5623-5668]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Rejecting an invalid JUMP at waypoint 1 returns before the existing mission is reset, so stale valid mission state remains available to execute.
## Fix Focus Areas
- src/main/navigation/navigation.c[5471-5483]
- src/main/fc/fc_msp.c[3323-3354]
## Recommended Fix
Reorder first-waypoint handling so `resetWaypointList()` runs before JUMP validation when `wpNumber == 1`, while still validating before copying or converting the new waypoint. Ensure a rejected first waypoint leaves the count zero and `waypointListValid` false.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Tip of the day

💡 Did you know, you can keep summaries lean with Finding overflow, which tucks the rest behind 'View more'

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

github-actions · 2026-09-23T13:09:22Z

RAM / Flash usage vs. base commit 76ee415 — commit 3645ed5

Target	Flash Δ	RAM Δ
MATEKF405 ⚠️	+23188 B (+3.32%)	-10396 B (-6.96%)
MATEKF722 ⚠️	+10380 B (+2.21%)	-11256 B (-8.98%)
MATEKF765 ⚠️	+16092 B (+2.18%)	-9264 B (-5.60%)
MATEKH743 ⚠️	+22816 B (+2.94%)	-8444 B (-4.99%)

See RAM/flash optimization guide for techniques to reduce usage.

github-actions · 2026-09-23T13:09:51Z

Test firmware build ready — commit 3645ed5

Download firmware for PR #11900

250 targets built. Find your board's .hex file by name on that page (e.g. MATEKF405SE.hex). Files are individually downloadable — no GitHub login required.

Development build for testing only. Use Full Chip Erase when flashing.

Raffi1202 marked this pull request as ready for review September 11, 2026 16:02

qodo-free-for-open-source-projects Bot reviewed Sep 11, 2026

View reviewed changes

Comment thread src/main/navigation/navigation.c Outdated

Invalidate the old mission before rejecting its replacement

11a226b

Raffi1202 mentioned this pull request Sep 13, 2026

[TEST] CI build for iNavFlight/inav#11900 (do not merge) Raffi1202/inav#14

Closed

sensei-hacker added this to the 10.0 milestone Sep 20, 2026

Re-run CI

3645ed5

Navigation Menu

Reject JUMP waypoints with an out-of-range target - #11900

Reject JUMP waypoints with an out-of-range target#11900
Raffi1202 wants to merge 3 commits into
iNavFlight:maintenance-10.xfrom
Raffi1202:fix/wp-jump-target-bounds

Raffi1202 commented Sep 10, 2026 •

edited

Loading

Uh oh!

qodo-code-review Bot commented Sep 11, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 11, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 11, 2026 •

edited

Loading

Uh oh!

Uh oh!

github-actions Bot commented Sep 23, 2026

Uh oh!

github-actions Bot commented Sep 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Raffi1202 commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Cause

Change

Test

Flash / RAM

Docs

Uh oh!

qodo-code-review Bot commented Sep 11, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 11, 2026

PR Summary by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

github-actions Bot commented Sep 23, 2026

Uh oh!

github-actions Bot commented Sep 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Raffi1202 commented Sep 10, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Sep 11, 2026 •

edited

Loading