Skip to content

Commit 18af8b3

Browse files
sensei-hackersensei-hacker
authored
Merge pull request #11365 from sensei-hacker/fix/pr10048-add-msg
[FIX] Prevent stack smashing via unbounded OSD message array writes
2 parents b3109d1 + 06fb807 commit 18af8b3

1 file changed

Lines changed: 56 additions & 54 deletions

File tree

‎src/main/io/osd.c‎

Lines changed: 56 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -6156,10 +6156,10 @@ textAttributes_t osdGetSystemMessage(char *buff, size_t buff_size, bool isCenter
61566156
/* WARNING: messageBuf is shared, use accordingly */
61576157
char messageBuf[MAX(SETTING_MAX_NAME_LENGTH, OSD_MESSAGE_LENGTH + 1)];
61586158

6159-
/* WARNING: ensure number of messages returned does not exceed messages array size
6160-
* Messages array set 1 larger than maximum expected message count of 7 */
6159+
/* Messages array - use ADD_MSG() for bounds-safe access. */
61616160
const char *messages[8];
61626161
unsigned messageCount = 0;
6162+
#define ADD_MSG(msg) do { if (messageCount < ARRAYLEN(messages)) messages[messageCount++] = (msg); } while(0)
61636163

61646164
const char *failsafeInfoMessage = NULL;
61656165
const char *invertedInfoMessage = NULL;
@@ -6168,16 +6168,16 @@ textAttributes_t osdGetSystemMessage(char *buff, size_t buff_size, bool isCenter
61686168
if (FLIGHT_MODE(FAILSAFE_MODE) || FLIGHT_MODE(NAV_RTH_MODE) || FLIGHT_MODE(NAV_WP_MODE) || navigationIsExecutingAnEmergencyLanding()) {
61696169
/* ADDS MAXIMUM OF 3 MESSAGES TO TOTAL NORMALLY, 5 MESSAGES DURING FAILSAFE */
61706170
if (navGetCurrentStateFlags() & NAV_AUTO_WP_DONE) {
6171-
messages[messageCount++] = STATE(LANDING_DETECTED) ? OSD_MESSAGE_STR(OSD_MSG_WP_LANDED) : OSD_MESSAGE_STR(OSD_MSG_WP_FINISHED);
6171+
ADD_MSG(STATE(LANDING_DETECTED) ? OSD_MESSAGE_STR(OSD_MSG_WP_LANDED) : OSD_MESSAGE_STR(OSD_MSG_WP_FINISHED));
61726172
} else if (NAV_Status.state == MW_NAV_STATE_WP_ENROUTE) {
61736173
// Countdown display for remaining Waypoints
61746174
char buf[6];
61756175
osdFormatDistanceSymbol(buf, posControl.wpDistance, 0, 3);
61766176
tfp_sprintf(messageBuf, "TO WP %u/%u (%s)", getGeoWaypointNumber(posControl.activeWaypointIndex), posControl.geoWaypointCount, buf);
6177-
messages[messageCount++] = messageBuf;
6177+
ADD_MSG(messageBuf);
61786178
} else if (NAV_Status.state == MW_NAV_STATE_HOLD_TIMED) {
61796179
if (navConfig()->general.waypoint_enforce_altitude && !posControl.wpAltitudeReached) {
6180-
messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_ADJUSTING_WP_ALT);
6180+
ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_ADJUSTING_WP_ALT));
61816181
} else {
61826182
// WP hold time countdown in seconds
61836183
timeMs_t currentTime = millis();
@@ -6186,66 +6186,66 @@ textAttributes_t osdGetSystemMessage(char *buff, size_t buff_size, bool isCenter
61866186

61876187
tfp_sprintf(messageBuf, "HOLDING WP FOR %2u S", holdTimeRemaining);
61886188

6189-
messages[messageCount++] = messageBuf;
6189+
ADD_MSG(messageBuf);
61906190
}
61916191
}
61926192
else {
61936193
const char *navStateMessage = navigationStateMessage();
61946194
if (navStateMessage) {
6195-
messages[messageCount++] = navStateMessage;
6195+
ADD_MSG(navStateMessage);
61966196
}
61976197
}
61986198
#if defined(USE_SAFE_HOME)
61996199
const char *safehomeMessage = divertingToSafehomeMessage();
62006200
if (safehomeMessage) {
6201-
messages[messageCount++] = safehomeMessage;
6201+
ADD_MSG(safehomeMessage);
62026202
}
62036203
#endif
62046204

62056205
#ifdef USE_GEOZONE
62066206
if (geozone.avoidInRTHInProgress) {
6207-
messages[messageCount++] = OSD_MSG_AVOID_ZONES_RTH;
6207+
ADD_MSG(OSD_MSG_AVOID_ZONES_RTH);
62086208
}
62096209
#endif
62106210
if (FLIGHT_MODE(FAILSAFE_MODE)) { // In FS mode while armed
62116211
if (NAV_Status.state == MW_NAV_STATE_LAND_SETTLE && posControl.landingDelay > 0) {
62126212
uint16_t remainingHoldSec = MS2S(posControl.landingDelay - millis());
62136213
tfp_sprintf(messageBuf, "LANDING DELAY: %3u SECONDS", remainingHoldSec);
62146214

6215-
messages[messageCount++] = messageBuf;
6215+
ADD_MSG(messageBuf);
62166216
}
62176217

62186218
const char *failsafePhaseMessage = osdFailsafePhaseMessage();
62196219
failsafeInfoMessage = osdFailsafeInfoMessage();
62206220

62216221
if (failsafePhaseMessage) {
6222-
messages[messageCount++] = failsafePhaseMessage;
6222+
ADD_MSG(failsafePhaseMessage);
62236223
}
62246224
if (failsafeInfoMessage) {
6225-
messages[messageCount++] = failsafeInfoMessage;
6225+
ADD_MSG(failsafeInfoMessage);
62266226
}
62276227
} else if (isWaypointMissionRTHActive()) {
62286228
// if RTH activated whilst WP mode selected, remind pilot to cancel WP mode to exit RTH
6229-
messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_WP_RTH_CANCEL);
6229+
ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_WP_RTH_CANCEL));
62306230
}
62316231
} else if (STATE(LANDING_DETECTED)) {
6232-
messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_LANDED);
6232+
ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_LANDED));
62336233
} else {
62346234
/* Messages shown only when Failsafe, WP, RTH or Emergency Landing not active and landed state inactive */
62356235
/* ADDS MAXIMUM OF 5 MESSAGES TO TOTAL */
62366236
#ifdef USE_GEOZONE
62376237
char buf[12], buf1[12];
62386238
switch (geozone.messageState) { /* ADDS MAXIMUM OF 2 MESSAGES TO TOTAL */
62396239
case GEOZONE_MESSAGE_STATE_NFZ:
6240-
messages[messageCount++] = OSD_MSG_NFZ;
6240+
ADD_MSG(OSD_MSG_NFZ);
62416241
break;
62426242
case GEOZONE_MESSAGE_STATE_LEAVING_FZ:
62436243
osdFormatDistanceSymbol(buf, geozone.distanceToZoneBorder3d, 0, 3);
62446244
tfp_sprintf(messageBuf, OSD_MSG_LEAVING_FZ, buf);
6245-
messages[messageCount++] = messageBuf;
6245+
ADD_MSG(messageBuf);
62466246
break;
62476247
case GEOZONE_MESSAGE_STATE_OUTSIDE_FZ:
6248-
messages[messageCount++] = OSD_MSG_OUTSIDE_FZ;
6248+
ADD_MSG(OSD_MSG_OUTSIDE_FZ);
62496249
break;
62506250
case GEOZONE_MESSAGE_STATE_ENTERING_NFZ:
62516251
osdFormatDistanceSymbol(buf, geozone.distanceToZoneBorder3d, 0, 3);
@@ -6255,36 +6255,36 @@ textAttributes_t osdGetSystemMessage(char *buff, size_t buff_size, bool isCenter
62556255
osdFormatAltitudeSymbol(buf1, geozone.zoneInfo);
62566256
}
62576257
tfp_sprintf(messageBuf, OSD_MSG_ENTERING_NFZ, buf, buf1);
6258-
messages[messageCount++] = messageBuf;
6258+
ADD_MSG(messageBuf);
62596259
break;
62606260
case GEOZONE_MESSAGE_STATE_AVOIDING_FB:
6261-
messages[messageCount++] = OSD_MSG_AVOIDING_FB;
6261+
ADD_MSG(OSD_MSG_AVOIDING_FB);
62626262
if (!posControl.sendTo.lockSticks) {
6263-
messages[messageCount++] = OSD_MSG_MOVE_STICKS;
6263+
ADD_MSG(OSD_MSG_MOVE_STICKS);
62646264
}
62656265
break;
62666266
case GEOZONE_MESSAGE_STATE_RETURN_TO_ZONE:
6267-
messages[messageCount++] = OSD_MSG_RETURN_TO_ZONE;
6267+
ADD_MSG(OSD_MSG_RETURN_TO_ZONE);
62686268
if (!posControl.sendTo.lockSticks) {
6269-
messages[messageCount++] = OSD_MSG_MOVE_STICKS;
6269+
ADD_MSG(OSD_MSG_MOVE_STICKS);
62706270
}
62716271
break;
62726272
case GEOZONE_MESSAGE_STATE_AVOIDING_ALTITUDE_BREACH:
6273-
messages[messageCount++] = OSD_MSG_AVOIDING_ALT_BREACH;
6273+
ADD_MSG(OSD_MSG_AVOIDING_ALT_BREACH);
62746274
if (!posControl.sendTo.lockSticks) {
6275-
messages[messageCount++] = OSD_MSG_MOVE_STICKS;
6275+
ADD_MSG(OSD_MSG_MOVE_STICKS);
62766276
}
62776277
break;
62786278
case GEOZONE_MESSAGE_STATE_FLYOUT_NFZ:
6279-
messages[messageCount++] = OSD_MSG_FLYOUT_NFZ;
6279+
ADD_MSG(OSD_MSG_FLYOUT_NFZ);
62806280
if (!posControl.sendTo.lockSticks) {
6281-
messages[messageCount++] = OSD_MSG_MOVE_STICKS;
6281+
ADD_MSG(OSD_MSG_MOVE_STICKS);
62826282
}
62836283
break;
62846284
case GEOZONE_MESSAGE_STATE_POS_HOLD:
6285-
messages[messageCount++] = OSD_MSG_AVOIDING_FB;
6285+
ADD_MSG(OSD_MSG_AVOIDING_FB);
62866286
if (!geozone.sticksLocked) {
6287-
messages[messageCount++] = OSD_MSG_MOVE_STICKS;
6287+
ADD_MSG(OSD_MSG_MOVE_STICKS);
62886288
}
62896289
break;
62906290
case GEOZONE_MESSAGE_STATE_NONE:
@@ -6294,37 +6294,37 @@ textAttributes_t osdGetSystemMessage(char *buff, size_t buff_size, bool isCenter
62946294
if (STATE(AIRPLANE)) { /* ADDS MAXIMUM OF 3 MESSAGES TO TOTAL */
62956295
#ifdef USE_FW_AUTOLAND
62966296
if (canFwLandingBeCancelled()) {
6297-
messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_MOVE_STICKS);
6297+
ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_MOVE_STICKS));
62986298
} else
62996299
#endif
63006300
if (navGetCurrentStateFlags() & NAV_CTL_LAUNCH) {
6301-
messages[messageCount++] = navConfig()->fw.launch_manual_throttle ? OSD_MESSAGE_STR(OSD_MSG_AUTOLAUNCH_MANUAL) :
6302-
OSD_MESSAGE_STR(OSD_MSG_AUTOLAUNCH);
6301+
ADD_MSG(navConfig()->fw.launch_manual_throttle ? OSD_MESSAGE_STR(OSD_MSG_AUTOLAUNCH_MANUAL) :
6302+
OSD_MESSAGE_STR(OSD_MSG_AUTOLAUNCH));
63036303
const char *launchStateMessage = fixedWingLaunchStateMessage();
63046304
if (launchStateMessage) {
6305-
messages[messageCount++] = launchStateMessage;
6305+
ADD_MSG(launchStateMessage);
63066306
}
63076307
} else if (FLIGHT_MODE(SOARING_MODE)) {
6308-
messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_NAV_SOARING);
6308+
ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_NAV_SOARING));
63096309
} else if (isFwAutoModeActive(BOXAUTOTUNE)) {
6310-
messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_AUTOTUNE);
6310+
ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_AUTOTUNE));
63116311
if (FLIGHT_MODE(MANUAL_MODE)) {
6312-
messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_AUTOTUNE_ACRO);
6312+
ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_AUTOTUNE_ACRO));
63136313
}
63146314
} else if (isFwAutoModeActive(BOXAUTOTRIM) && !feature(FEATURE_FW_AUTOTRIM)) {
6315-
messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_AUTOTRIM);
6315+
ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_AUTOTRIM));
63166316
} else if (isFixedWingLevelTrimActive()) {
6317-
messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_AUTOLEVEL);
6317+
ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_AUTOLEVEL));
63186318
}
63196319

63206320
if (IS_RC_MODE_ACTIVE(BOXANGLEHOLD)) {
63216321
int8_t navAngleHoldAxis = navCheckActiveAngleHoldAxis();
63226322
if (isAngleHoldLevel()) {
6323-
messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_ANGLEHOLD_LEVEL);
6323+
ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_ANGLEHOLD_LEVEL));
63246324
} else if (navAngleHoldAxis == FD_ROLL) {
6325-
messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_ANGLEHOLD_ROLL);
6325+
ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_ANGLEHOLD_ROLL));
63266326
} else if (navAngleHoldAxis == FD_PITCH) {
6327-
messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_ANGLEHOLD_PITCH);
6327+
ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_ANGLEHOLD_PITCH));
63286328
}
63296329
}
63306330
} else if (STATE(MULTIROTOR)) { /* ADDS MAXIMUM OF 2 MESSAGES TO TOTAL */
@@ -6336,25 +6336,26 @@ textAttributes_t osdGetSystemMessage(char *buff, size_t buff_size, bool isCenter
63366336
} else {
63376337
strcpy(messageBuf, "(HOLD)");
63386338
}
6339-
messages[messageCount++] = messageBuf;
6339+
ADD_MSG(messageBuf);
63406340
} else if (FLIGHT_MODE(HEADFREE_MODE)) {
6341-
messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_HEADFREE);
6341+
ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_HEADFREE));
63426342
}
63436343

63446344
if (FLIGHT_MODE(NAV_ALTHOLD_MODE)) {
63456345
if (posControl.flags.isTerrainFollowEnabled) {
63466346
if (posControl.flags.estAglStatus == EST_TRUSTED) {
6347-
messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_SURFACE_OK);
6347+
ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_SURFACE_OK));
63486348
} else {
6349-
messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_SURFACE_BAD);
6349+
ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_SURFACE_BAD));
63506350
}
63516351
} else if (!navigationRequiresAngleMode()) {
63526352
/* If ALTHOLD is separately enabled for multirotor together with ANGL/HORIZON/ACRO modes
63536353
* then ANGL/HORIZON/ACRO are indicated by the OSD_FLYMODE field.
63546354
* In this case indicate ALTHOLD is active via a system message */
63556355

6356-
messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_ALTITUDE_HOLD);
6356+
ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_ALTITUDE_HOLD));
63576357
}
6358+
63586359
}
63596360
}
63606361
}
@@ -6369,44 +6370,44 @@ textAttributes_t osdGetSystemMessage(char *buff, size_t buff_size, bool isCenter
63696370
messageBuf[ii] = sl_toupper(messageBuf[ii]);
63706371
}
63716372
invertedInfoMessage = messageBuf;
6372-
messages[messageCount++] = invertedInfoMessage;
6373+
ADD_MSG(invertedInfoMessage);
63736374

63746375
invertedInfoMessage = OSD_MESSAGE_STR(OSD_MSG_INVALID_SETTING);
6375-
messages[messageCount++] = invertedInfoMessage;
6376+
ADD_MSG(invertedInfoMessage);
63766377
} else {
63776378
invertedInfoMessage = OSD_MESSAGE_STR(OSD_MSG_UNABLE_ARM);
6378-
messages[messageCount++] = invertedInfoMessage;
6379+
ADD_MSG(invertedInfoMessage);
63796380
// Show the reason for not arming
6380-
messages[messageCount++] = osdArmingDisabledReasonMessage();
6381+
ADD_MSG(osdArmingDisabledReasonMessage());
63816382
}
63826383
} else if (!ARMING_FLAG(ARMED)) {
63836384
if (isWaypointListValid()) {
6384-
messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_WP_MISSION_LOADED);
6385+
ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_WP_MISSION_LOADED));
63856386
}
63866387
}
63876388

63886389
/* Messages that are shown regardless of Arming state */
63896390
/* ADDS MAXIMUM OF 2 MESSAGES TO TOTAL NORMALLY, 1 MESSAGE DURING FAILSAFE */
63906391
if (posControl.flags.wpMissionPlannerActive && !FLIGHT_MODE(FAILSAFE_MODE)) {
6391-
messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_MISSION_PLANNER);
6392+
ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_MISSION_PLANNER));
63926393
}
63936394

63946395
// The following has been commented out as it will be added in #9688
63956396
// uint16_t rearmMs = (emergInflightRearmEnabled()) ? emergencyInFlightRearmTimeMS() : 0;
63966397

63976398
if (savingSettings == true) {
6398-
messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_SAVING_SETTNGS);
6399+
ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_SAVING_SETTNGS));
63996400
/*} else if (rearmMs > 0) { // Show rearming time if settings not actively being saved. Ignore the settings saved message if rearm available.
64006401
char emReArmMsg[23];
64016402
tfp_sprintf(emReArmMsg, "** REARM PERIOD: ");
64026403
tfp_sprintf(emReArmMsg + strlen(emReArmMsg), "%02d", (uint8_t)MS2S(rearmMs));
64036404
strcat(emReArmMsg, " **\0");
6404-
messages[messageCount++] = OSD_MESSAGE_STR(emReArmMsg);*/
6405+
ADD_MSG(OSD_MESSAGE_STR(emReArmMsg));*/
64056406
} else if (notify_settings_saved > 0) {
64066407
if (millis() > notify_settings_saved) {
64076408
notify_settings_saved = 0;
64086409
} else {
6409-
messages[messageCount++] = OSD_MESSAGE_STR(OSD_MSG_SETTINGS_SAVED);
6410+
ADD_MSG(OSD_MESSAGE_STR(OSD_MSG_SETTINGS_SAVED));
64106411
}
64116412
}
64126413

@@ -6429,6 +6430,7 @@ textAttributes_t osdGetSystemMessage(char *buff, size_t buff_size, bool isCenter
64296430

64306431
osdFormatMessage(buff, buff_size, message, isCenteredText);
64316432
}
6433+
#undef ADD_MSG
64326434
return elemAttr;
64336435
}
64346436

0 commit comments

Comments
 (0)