From 1938d43e75d393bc12fd27cb8d934402f4ec7426 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Wed, 25 Feb 2026 12:32:07 -0600 Subject: [PATCH 1/2] [FIX] Prevent stack smashing via unbounded OSD message array writes Both osdGetSystemMessage() and osdGetMultiFunctionMessage() accumulate message pointers into a fixed-size array with no bounds checking. With recent additions (geozone avoidance, pitot validation, etc.) the number of potential writes exceeds the array size, causing stack corruption. Introduce a scoped ADD_MSG() macro that guards each write: #define ADD_MSG(msg) if (messageCount < ARRAYLEN(messages)) \ messages[messageCount++] = (msg) Excess messages are silently dropped rather than written past the end of the array. The macro is #undef'd at the end of each function. Fixes: https://github.com/iNavFlight/inav/pull/10048 (Yury-MonZon) --- src/main/io/osd.c | 123 ++++++++++++++++++++++++---------------------- 1 file changed, 63 insertions(+), 60 deletions(-) diff --git a/src/main/io/osd.c b/src/main/io/osd.c index 6ad55632c17..722648d10fd 100644 --- a/src/main/io/osd.c +++ b/src/main/io/osd.c @@ -6137,10 +6137,10 @@ textAttributes_t osdGetSystemMessage(char *buff, size_t buff_size, bool isCenter /* WARNING: messageBuf is shared, use accordingly */ char messageBuf[MAX(SETTING_MAX_NAME_LENGTH, OSD_MESSAGE_LENGTH + 1)]; - /* WARNING: ensure number of messages returned does not exceed messages array size - * Messages array set 1 larger than maximum expected message count of 6 */ + /* Messages array - use ADD_MSG() for bounds-safe access. */ const char *messages[7]; unsigned messageCount = 0; + #define ADD_MSG(msg) if (messageCount < ARRAYLEN(messages)) messages[messageCount++] = (msg) const char *failsafeInfoMessage = NULL; const char *invertedInfoMessage = NULL; @@ -6149,16 +6149,16 @@ textAttributes_t osdGetSystemMessage(char *buff, size_t buff_size, bool isCenter if (FLIGHT_MODE(FAILSAFE_MODE) || FLIGHT_MODE(NAV_RTH_MODE) || FLIGHT_MODE(NAV_WP_MODE) || navigationIsExecutingAnEmergencyLanding()) { /* ADDS MAXIMUM OF 3 MESSAGES TO TOTAL NORMALLY, 5 MESSAGES DURING FAILSAFE */ if (navGetCurrentStateFlags() & NAV_AUTO_WP_DONE) { - messages[messageCount++] = STATE(LANDING_DETECTED) ? OSD_MESSAGE_STR(OSD_MSG_WP_LANDED) : OSD_MESSAGE_STR(OSD_MSG_WP_FINISHED); + ADD_MSG(STATE(LANDING_DETECTED) ? OSD_MESSAGE_STR(OSD_MSG_WP_LANDED) : OSD_MESSAGE_STR(OSD_MSG_WP_FINISHED)); } else if (NAV_Status.state == MW_NAV_STATE_WP_ENROUTE) { // Countdown display for remaining Waypoints char buf[6]; osdFormatDistanceSymbol(buf, posControl.wpDistance, 0, 3); tfp_sprintf(messageBuf, "TO WP %u/%u (%s)", getGeoWaypointNumber(posControl.activeWaypointIndex), posControl.geoWaypointCount, buf); - messages[messageCount++] = messageBuf; + ADD_MSG(messageBuf); } else if (NAV_Status.state == MW_NAV_STATE_HOLD_TIMED) { if (navConfig()->general.waypoint_enforce_altitude && !posControl.wpAltitudeReached) { - messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_ADJUSTING_WP_ALT); + ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_ADJUSTING_WP_ALT)); } else { // WP hold time countdown in seconds timeMs_t currentTime = millis(); @@ -6167,25 +6167,25 @@ textAttributes_t osdGetSystemMessage(char *buff, size_t buff_size, bool isCenter tfp_sprintf(messageBuf, "HOLDING WP FOR %2u S", holdTimeRemaining); - messages[messageCount++] = messageBuf; + ADD_MSG(messageBuf); } } else { const char *navStateMessage = navigationStateMessage(); if (navStateMessage) { - messages[messageCount++] = navStateMessage; + ADD_MSG(navStateMessage); } } #if defined(USE_SAFE_HOME) const char *safehomeMessage = divertingToSafehomeMessage(); if (safehomeMessage) { - messages[messageCount++] = safehomeMessage; + ADD_MSG(safehomeMessage); } #endif #ifdef USE_GEOZONE if (geozone.avoidInRTHInProgress) { - messages[messageCount++] = OSD_MSG_AVOID_ZONES_RTH; + ADD_MSG(OSD_MSG_AVOID_ZONES_RTH); } #endif if (FLIGHT_MODE(FAILSAFE_MODE)) { // In FS mode while armed @@ -6193,38 +6193,38 @@ textAttributes_t osdGetSystemMessage(char *buff, size_t buff_size, bool isCenter uint16_t remainingHoldSec = MS2S(posControl.landingDelay - millis()); tfp_sprintf(messageBuf, "LANDING DELAY: %3u SECONDS", remainingHoldSec); - messages[messageCount++] = messageBuf; + ADD_MSG(messageBuf); } const char *failsafePhaseMessage = osdFailsafePhaseMessage(); failsafeInfoMessage = osdFailsafeInfoMessage(); if (failsafePhaseMessage) { - messages[messageCount++] = failsafePhaseMessage; + ADD_MSG(failsafePhaseMessage); } if (failsafeInfoMessage) { - messages[messageCount++] = failsafeInfoMessage; + ADD_MSG(failsafeInfoMessage); } } else if (isWaypointMissionRTHActive()) { // if RTH activated whilst WP mode selected, remind pilot to cancel WP mode to exit RTH - messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_WP_RTH_CANCEL); + ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_WP_RTH_CANCEL)); } } else if (STATE(LANDING_DETECTED)) { - messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_LANDED); + ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_LANDED)); } else { #ifdef USE_GEOZONE char buf[12], buf1[12]; switch (geozone.messageState) { case GEOZONE_MESSAGE_STATE_NFZ: - messages[messageCount++] = OSD_MSG_NFZ; + ADD_MSG(OSD_MSG_NFZ); break; case GEOZONE_MESSAGE_STATE_LEAVING_FZ: osdFormatDistanceSymbol(buf, geozone.distanceToZoneBorder3d, 0, 3); tfp_sprintf(messageBuf, OSD_MSG_LEAVING_FZ, buf); - messages[messageCount++] = messageBuf; + ADD_MSG(messageBuf); break; case GEOZONE_MESSAGE_STATE_OUTSIDE_FZ: - messages[messageCount++] = OSD_MSG_OUTSIDE_FZ; + ADD_MSG(OSD_MSG_OUTSIDE_FZ); break; case GEOZONE_MESSAGE_STATE_ENTERING_NFZ: osdFormatDistanceSymbol(buf, geozone.distanceToZoneBorder3d, 0, 3); @@ -6234,36 +6234,36 @@ textAttributes_t osdGetSystemMessage(char *buff, size_t buff_size, bool isCenter osdFormatAltitudeSymbol(buf1, geozone.zoneInfo); } tfp_sprintf(messageBuf, OSD_MSG_ENTERING_NFZ, buf, buf1); - messages[messageCount++] = messageBuf; + ADD_MSG(messageBuf); break; case GEOZONE_MESSAGE_STATE_AVOIDING_FB: - messages[messageCount++] = OSD_MSG_AVOIDING_FB; + ADD_MSG(OSD_MSG_AVOIDING_FB); if (!posControl.sendTo.lockSticks) { - messages[messageCount++] = OSD_MSG_MOVE_STICKS; + ADD_MSG(OSD_MSG_MOVE_STICKS); } break; case GEOZONE_MESSAGE_STATE_RETURN_TO_ZONE: - messages[messageCount++] = OSD_MSG_RETURN_TO_ZONE; + ADD_MSG(OSD_MSG_RETURN_TO_ZONE); if (!posControl.sendTo.lockSticks) { - messages[messageCount++] = OSD_MSG_MOVE_STICKS; + ADD_MSG(OSD_MSG_MOVE_STICKS); } break; case GEOZONE_MESSAGE_STATE_AVOIDING_ALTITUDE_BREACH: - messages[messageCount++] = OSD_MSG_AVOIDING_ALT_BREACH; + ADD_MSG(OSD_MSG_AVOIDING_ALT_BREACH); if (!posControl.sendTo.lockSticks) { - messages[messageCount++] = OSD_MSG_MOVE_STICKS; + ADD_MSG(OSD_MSG_MOVE_STICKS); } break; case GEOZONE_MESSAGE_STATE_FLYOUT_NFZ: - messages[messageCount++] = OSD_MSG_FLYOUT_NFZ; + ADD_MSG(OSD_MSG_FLYOUT_NFZ); if (!posControl.sendTo.lockSticks) { - messages[messageCount++] = OSD_MSG_MOVE_STICKS; + ADD_MSG(OSD_MSG_MOVE_STICKS); } break; case GEOZONE_MESSAGE_STATE_POS_HOLD: - messages[messageCount++] = OSD_MSG_AVOIDING_FB; + ADD_MSG(OSD_MSG_AVOIDING_FB); if (!geozone.sticksLocked) { - messages[messageCount++] = OSD_MSG_MOVE_STICKS; + ADD_MSG(OSD_MSG_MOVE_STICKS); } break; case GEOZONE_MESSAGE_STATE_NONE: @@ -6275,37 +6275,37 @@ textAttributes_t osdGetSystemMessage(char *buff, size_t buff_size, bool isCenter if (STATE(AIRPLANE)) { /* ADDS MAXIMUM OF 3 MESSAGES TO TOTAL */ #ifdef USE_FW_AUTOLAND if (canFwLandingBeCancelled()) { - messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_MOVE_STICKS); + ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_MOVE_STICKS)); } else #endif if (navGetCurrentStateFlags() & NAV_CTL_LAUNCH) { - messages[messageCount++] = navConfig()->fw.launch_manual_throttle ? OSD_MESSAGE_STR(OSD_MSG_AUTOLAUNCH_MANUAL) : - OSD_MESSAGE_STR(OSD_MSG_AUTOLAUNCH); + ADD_MSG(navConfig()->fw.launch_manual_throttle ? OSD_MESSAGE_STR(OSD_MSG_AUTOLAUNCH_MANUAL) : + OSD_MESSAGE_STR(OSD_MSG_AUTOLAUNCH)); const char *launchStateMessage = fixedWingLaunchStateMessage(); if (launchStateMessage) { - messages[messageCount++] = launchStateMessage; + ADD_MSG(launchStateMessage); } } else if (FLIGHT_MODE(SOARING_MODE)) { - messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_NAV_SOARING); + ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_NAV_SOARING)); } else if (isFwAutoModeActive(BOXAUTOTUNE)) { - messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_AUTOTUNE); + ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_AUTOTUNE)); if (FLIGHT_MODE(MANUAL_MODE)) { - messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_AUTOTUNE_ACRO); + ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_AUTOTUNE_ACRO)); } } else if (isFwAutoModeActive(BOXAUTOTRIM) && !feature(FEATURE_FW_AUTOTRIM)) { - messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_AUTOTRIM); + ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_AUTOTRIM)); } else if (isFixedWingLevelTrimActive()) { - messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_AUTOLEVEL); + ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_AUTOLEVEL)); } if (IS_RC_MODE_ACTIVE(BOXANGLEHOLD)) { int8_t navAngleHoldAxis = navCheckActiveAngleHoldAxis(); if (isAngleHoldLevel()) { - messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_ANGLEHOLD_LEVEL); + ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_ANGLEHOLD_LEVEL)); } else if (navAngleHoldAxis == FD_ROLL) { - messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_ANGLEHOLD_ROLL); + ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_ANGLEHOLD_ROLL)); } else if (navAngleHoldAxis == FD_PITCH) { - messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_ANGLEHOLD_PITCH); + ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_ANGLEHOLD_PITCH)); } } @@ -6318,16 +6318,16 @@ textAttributes_t osdGetSystemMessage(char *buff, size_t buff_size, bool isCenter } else { strcpy(messageBuf, "(HOLD)"); } - messages[messageCount++] = messageBuf; + ADD_MSG(messageBuf); } else if (FLIGHT_MODE(HEADFREE_MODE)) { - messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_HEADFREE); + ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_HEADFREE)); } if (FLIGHT_MODE(NAV_ALTHOLD_MODE) && !navigationRequiresAngleMode()) { /* If ALTHOLD is separately enabled for multirotor together with ANGL/HORIZON/ACRO modes * then ANGL/HORIZON/ACRO are indicated by the OSD_FLYMODE field. * In this case indicate ALTHOLD is active via a system message */ - messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_ALTITUDE_HOLD); + ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_ALTITUDE_HOLD)); } } } @@ -6342,44 +6342,44 @@ textAttributes_t osdGetSystemMessage(char *buff, size_t buff_size, bool isCenter messageBuf[ii] = sl_toupper(messageBuf[ii]); } invertedInfoMessage = messageBuf; - messages[messageCount++] = invertedInfoMessage; + ADD_MSG(invertedInfoMessage); invertedInfoMessage = OSD_MESSAGE_STR(OSD_MSG_INVALID_SETTING); - messages[messageCount++] = invertedInfoMessage; + ADD_MSG(invertedInfoMessage); } else { invertedInfoMessage = OSD_MESSAGE_STR(OSD_MSG_UNABLE_ARM); - messages[messageCount++] = invertedInfoMessage; + ADD_MSG(invertedInfoMessage); // Show the reason for not arming - messages[messageCount++] = osdArmingDisabledReasonMessage(); + ADD_MSG(osdArmingDisabledReasonMessage()); } } else if (!ARMING_FLAG(ARMED)) { if (isWaypointListValid()) { - messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_WP_MISSION_LOADED); + ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_WP_MISSION_LOADED)); } } /* Messages that are shown regardless of Arming state */ /* ADDS MAXIMUM OF 2 MESSAGES TO TOTAL NORMALLY, 1 MESSAGE DURING FAILSAFE */ if (posControl.flags.wpMissionPlannerActive && !FLIGHT_MODE(FAILSAFE_MODE)) { - messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_MISSION_PLANNER); + ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_MISSION_PLANNER)); } // The following has been commented out as it will be added in #9688 // uint16_t rearmMs = (emergInflightRearmEnabled()) ? emergencyInFlightRearmTimeMS() : 0; if (savingSettings == true) { - messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_SAVING_SETTNGS); + ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_SAVING_SETTNGS)); /*} else if (rearmMs > 0) { // Show rearming time if settings not actively being saved. Ignore the settings saved message if rearm available. char emReArmMsg[23]; tfp_sprintf(emReArmMsg, "** REARM PERIOD: "); tfp_sprintf(emReArmMsg + strlen(emReArmMsg), "%02d", (uint8_t)MS2S(rearmMs)); strcat(emReArmMsg, " **\0"); - messages[messageCount++] = OSD_MESSAGE_STR(emReArmMsg);*/ + ADD_MSG(OSD_MESSAGE_STR(emReArmMsg));*/ } else if (notify_settings_saved > 0) { if (millis() > notify_settings_saved) { notify_settings_saved = 0; } else { - messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_SETTINGS_SAVED); + ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_SETTINGS_SAVED)); } } @@ -6402,6 +6402,7 @@ textAttributes_t osdGetSystemMessage(char *buff, size_t buff_size, bool isCenter osdFormatMessage(buff, buff_size, message, isCenteredText); } + #undef ADD_MSG return elemAttr; } @@ -6526,6 +6527,7 @@ static textAttributes_t osdGetMultiFunctionMessage(char *buff) /* --- WARNINGS --- */ const char *messages[7]; uint8_t messageCount = 0; + #define ADD_MSG(msg) if (messageCount < ARRAYLEN(messages)) messages[messageCount++] = (msg) bool warningCondition = false; warningsCount = 0; uint8_t warningFlagID = 1; @@ -6534,7 +6536,7 @@ static textAttributes_t osdGetMultiFunctionMessage(char *buff) const batteryState_e batteryState = getBatteryState(); warningCondition = batteryState == BATTERY_CRITICAL || batteryState == BATTERY_WARNING; if (osdCheckWarning(warningCondition, warningFlagID, &warningsCount)) { - messages[messageCount++] = batteryState == BATTERY_CRITICAL ? "BATT EMPTY" : "BATT LOW !"; + ADD_MSG(batteryState == BATTERY_CRITICAL ? "BATT EMPTY" : "BATT LOW !"); } #if defined(USE_GPS) @@ -6542,7 +6544,7 @@ static textAttributes_t osdGetMultiFunctionMessage(char *buff) if (feature(FEATURE_GPS)) { if (osdCheckWarning(!STATE(GPS_FIX), warningFlagID <<= 1, &warningsCount)) { bool gpsFailed = getHwGPSStatus() == HW_SENSOR_UNAVAILABLE; - messages[messageCount++] = gpsFailed ? "GPS FAILED" : "NO GPS FIX"; + ADD_MSG(gpsFailed ? "GPS FAILED" : "NO GPS FIX"); } } @@ -6550,12 +6552,12 @@ static textAttributes_t osdGetMultiFunctionMessage(char *buff) warningCondition = NAV_Status.state == MW_NAV_STATE_RTH_ENROUTE && !posControl.flags.rthTrackbackActive && (posControl.homeDistance - posControl.rthSanityChecker.minimalDistanceToHome) > 20000; if (osdCheckWarning(warningCondition, warningFlagID <<= 1, &warningsCount)) { - messages[messageCount++] = "RTH SANITY"; + ADD_MSG("RTH SANITY"); } // Altitude sanity (warning if significant mismatch between estimated and GPS altitude) if (osdCheckWarning(posControl.flags.gpsCfEstimatedAltitudeMismatch, warningFlagID <<= 1, &warningsCount)) { - messages[messageCount++] = "ALT SANITY"; + ADD_MSG("ALT SANITY"); } #endif @@ -6564,7 +6566,7 @@ static textAttributes_t osdGetMultiFunctionMessage(char *buff) if (requestedSensors[SENSOR_INDEX_MAG] != MAG_NONE) { hardwareSensorStatus_e magStatus = getHwCompassStatus(); if (osdCheckWarning(magStatus == HW_SENSOR_UNAVAILABLE || magStatus == HW_SENSOR_UNHEALTHY, warningFlagID <<= 1, &warningsCount)) { - messages[messageCount++] = "MAG FAILED"; + ADD_MSG("MAG FAILED"); } } #endif @@ -6573,7 +6575,7 @@ static textAttributes_t osdGetMultiFunctionMessage(char *buff) // Pitot sensor validation failure (blocked/failed pitot tube) if (sensors(SENSOR_PITOT) && detectedSensors[SENSOR_INDEX_PITOT] != PITOT_VIRTUAL) { if (osdCheckWarning(pitotHasFailed(), warningFlagID <<= 1, &warningsCount)) { - messages[messageCount++] = "PITOT FAIL"; + ADD_MSG("PITOT FAIL"); } } #endif @@ -6582,12 +6584,12 @@ static textAttributes_t osdGetMultiFunctionMessage(char *buff) // const float vibrationLevel = accGetVibrationLevel(); // warningCondition = vibrationLevel > 1.5f; // if (osdCheckWarning(warningCondition, warningFlagID <<= 1, &warningsCount)) { - // messages[messageCount++] = vibrationLevel > 2.5f ? "BAD VIBRTN" : "VIBRATION!"; + // ADD_MSG(vibrationLevel > 2.5f ? "BAD VIBRTN" : "VIBRATION!"); // } #ifdef USE_DEV_TOOLS if (osdCheckWarning(systemConfig()->groundTestMode, warningFlagID <<= 1, &warningsCount)) { - messages[messageCount++] = "GRD TEST !"; + ADD_MSG("GRD TEST !"); } #endif @@ -6600,6 +6602,7 @@ static textAttributes_t osdGetMultiFunctionMessage(char *buff) tfp_sprintf(buff + 1, "%u ", warningsCount); } + #undef ADD_MSG return elemAttr; } From 83079aa64e35b8e08a1fd5bb7e17a80d445ecd9f Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Wed, 25 Feb 2026 14:21:38 -0600 Subject: [PATCH 2/2] Use do...while(0) in ADD_MSG macro to prevent dangling-else bugs --- src/main/io/osd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/io/osd.c b/src/main/io/osd.c index 722648d10fd..01604d12868 100644 --- a/src/main/io/osd.c +++ b/src/main/io/osd.c @@ -6140,7 +6140,7 @@ textAttributes_t osdGetSystemMessage(char *buff, size_t buff_size, bool isCenter /* Messages array - use ADD_MSG() for bounds-safe access. */ const char *messages[7]; unsigned messageCount = 0; - #define ADD_MSG(msg) if (messageCount < ARRAYLEN(messages)) messages[messageCount++] = (msg) + #define ADD_MSG(msg) do { if (messageCount < ARRAYLEN(messages)) messages[messageCount++] = (msg); } while(0) const char *failsafeInfoMessage = NULL; const char *invertedInfoMessage = NULL; @@ -6527,7 +6527,7 @@ static textAttributes_t osdGetMultiFunctionMessage(char *buff) /* --- WARNINGS --- */ const char *messages[7]; uint8_t messageCount = 0; - #define ADD_MSG(msg) if (messageCount < ARRAYLEN(messages)) messages[messageCount++] = (msg) + #define ADD_MSG(msg) do { if (messageCount < ARRAYLEN(messages)) messages[messageCount++] = (msg); } while(0) bool warningCondition = false; warningsCount = 0; uint8_t warningFlagID = 1;