Skip to content

Commit 9bb5ca4

Browse files
caveman99ndoo
andcommitted
Clamp truncated writes and harden the remaining fixed buffers
snprintf returns the length it would have written, so a truncated NMEA sentence made buf + len point past the buffer and bufsz - len underflow into a huge size for the checksum append. Clamp after each write. Also pulls in the rest of #11236: the two remaining Dropzone sprintf calls, the dead strcpy in mt_sprintf that wrote one byte past a zero-size allocation for an empty format, and the 10-byte errcode buffer that INT32_MIN overflows. Co-Authored-By: Andrew Yong <me@ndoo.sg>
1 parent 764e1e5 commit 9bb5ca4

4 files changed

Lines changed: 18 additions & 9 deletions

File tree

src/RedirectablePrint.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,6 @@ std::string RedirectablePrint::mt_sprintf(const std::string fmt_str, ...)
386386
va_list ap;
387387
while (1) {
388388
formatted.reset(new char[n]); /* Wrap the plain char array into the unique_ptr */
389-
strcpy(&formatted[0], fmt_str.c_str());
390389
va_start(ap, fmt_str);
391390
int final_n = vsnprintf(&formatted[0], n, fmt_str.c_str(), ap);
392391
va_end(ap);

src/gps/NMEAWPL.cpp

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@
55
#include <string.h>
66
#include <time.h>
77

8+
static uint32_t nmeaClamp(uint32_t len, size_t bufsz)
9+
{
10+
if (len >= bufsz)
11+
return bufsz > 0 ? (uint32_t)(bufsz - 1) : 0;
12+
return len;
13+
}
14+
815
static uint32_t nmeaChecksum(const char *buf)
916
{
1017
uint32_t chk = 0;
@@ -39,8 +46,9 @@ uint32_t printWPL(char *buf, size_t bufsz, const meshtastic_PositionLite &pos, c
3946
(abs(geoCoord.getLatitude()) - geoCoord.getDMSLatDeg() * 1e+7) * 6e-6, geoCoord.getDMSLatCP(),
4047
geoCoord.getDMSLonDeg(), (abs(geoCoord.getLongitude()) - geoCoord.getDMSLonDeg() * 1e+7) * 6e-6,
4148
geoCoord.getDMSLonCP(), name);
49+
len = nmeaClamp(len, bufsz);
4250
uint32_t chk = nmeaChecksum(buf);
43-
len += snprintf(buf + len, bufsz - len, "*%02X\r\n", chk);
51+
len = nmeaClamp(len + snprintf(buf + len, bufsz - len, "*%02X\r\n", chk), bufsz);
4452
return len;
4553
}
4654

@@ -52,8 +60,9 @@ uint32_t printWPL(char *buf, size_t bufsz, const meshtastic_Position &pos, const
5260
(abs(geoCoord.getLatitude()) - geoCoord.getDMSLatDeg() * 1e+7) * 6e-6, geoCoord.getDMSLatCP(),
5361
geoCoord.getDMSLonDeg(), (abs(geoCoord.getLongitude()) - geoCoord.getDMSLonDeg() * 1e+7) * 6e-6,
5462
geoCoord.getDMSLonCP(), name);
63+
len = nmeaClamp(len, bufsz);
5564
uint32_t chk = nmeaChecksum(buf);
56-
len += snprintf(buf + len, bufsz - len, "*%02X\r\n", chk);
65+
len = nmeaClamp(len + snprintf(buf + len, bufsz - len, "*%02X\r\n", chk), bufsz);
5766
return len;
5867
}
5968
/* -------------------------------------------
@@ -97,8 +106,9 @@ uint32_t printGGA(char *buf, size_t bufsz, const meshtastic_Position &pos)
97106
(abs(geoCoord.getLongitude()) - geoCoord.getDMSLonDeg() * 1e+7) * 6e-6, geoCoord.getDMSLonCP(), pos.fix_quality,
98107
pos.sats_in_view, pos.HDOP, geoCoord.getAltitude(), 'M', pos.altitude_geoidal_separation, 'M', 0, 0);
99108

109+
len = nmeaClamp(len, bufsz);
100110
uint32_t chk = nmeaChecksum(buf);
101-
len += snprintf(buf + len, bufsz - len, "*%02X\r\n", chk);
111+
len = nmeaClamp(len + snprintf(buf + len, bufsz - len, "*%02X\r\n", chk), bufsz);
102112
return len;
103113
}
104114

src/modules/DropzoneModule.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,11 @@ meshtastic_MeshPacket *DropzoneModule::sendConditions()
8282
auto windDirection = telemetry.variant.environment_metrics.wind_direction;
8383
auto temp = telemetry.variant.environment_metrics.temperature;
8484
auto baro = UnitConversions::HectoPascalToInchesOfMercury(telemetry.variant.environment_metrics.barometric_pressure);
85-
sprintf(replyStr, "%s @ %02d:%02d:%02dz\nWind %.2f kts @ %d°\nBaro %.2f inHg %.2f°C", dropzoneStatus, hour, min, sec,
86-
windSpeed, windDirection, baro, temp);
85+
snprintf(replyStr, sizeof(replyStr), "%s @ %02d:%02d:%02dz\nWind %.2f kts @ %d°\nBaro %.2f inHg %.2f°C", dropzoneStatus,
86+
hour, min, sec, windSpeed, windDirection, baro, temp);
8787
} else {
8888
LOG_ERROR("No sensor found");
89-
sprintf(replyStr, "%s @ %02d:%02d:%02d\nNo sensor found", dropzoneStatus, hour, min, sec);
89+
snprintf(replyStr, sizeof(replyStr), "%s @ %02d:%02d:%02d\nNo sensor found", dropzoneStatus, hour, min, sec);
9090
}
9191
LOG_DEBUG("Conditions reply: %s", replyStr);
9292
reply->decoded.payload.size = strlen(replyStr); // You must specify how many bytes are in the reply

src/platform/stm32wl/STM32_LittleFS.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,8 +272,8 @@ const char *dbg_strerr_lfs(int32_t err)
272272
return "LFS_ERR_NOMEM";
273273

274274
default:
275-
static char errcode[10];
276-
sprintf(errcode, "%ld", err);
275+
static char errcode[13];
276+
snprintf(errcode, sizeof(errcode), "%ld", err);
277277
return errcode;
278278
}
279279

0 commit comments

Comments
 (0)