fix: defensive buffer-write hardening (sprintf, mt_sprintf, NMEA checksum)

src/RedirectablePrint.cpp

@@ -386,7 +386,6 @@ std::string RedirectablePrint::mt_sprintf(const std::string fmt_str, ...)
     va_list ap;
     while (1) {
         formatted.reset(new char[n]); /* Wrap the plain char array into the unique_ptr */
-        strcpy(&formatted[0], fmt_str.c_str());
         va_start(ap, fmt_str);
         int final_n = vsnprintf(&formatted[0], n, fmt_str.c_str(), ap);
         va_end(ap);

src/gps/NMEAWPL.cpp

@@ -27,11 +27,19 @@ uint32_t printWPL(char *buf, size_t bufsz, const meshtastic_PositionLite &pos, c
                             (abs(geoCoord.getLatitude()) - geoCoord.getDMSLatDeg() * 1e+7) * 6e-6, geoCoord.getDMSLatCP(),
                             geoCoord.getDMSLonDeg(), (abs(geoCoord.getLongitude()) - geoCoord.getDMSLonDeg() * 1e+7) * 6e-6,
                             geoCoord.getDMSLonCP(), name);
+    // snprintf() returns the length that *would* have been written, which can exceed bufsz if
+    // truncated - clamp before using it as an index/size, or the checksum loop reads out of
+    // bounds and bufsz-len underflows into a huge size for the next snprintf().
+    if (len >= bufsz)
+        len = bufsz > 0 ? bufsz - 1 : 0;
+
     uint32_t chk = 0;
     for (uint32_t i = 1; i < len; i++) {
         chk ^= buf[i];
     }
     len += snprintf(buf + len, bufsz - len, "*%02X\r\n", chk);
+    if (len >= bufsz)
+        len = bufsz > 0 ? bufsz - 1 : 0;
     return len;
 }
 
@@ -43,11 +51,19 @@ uint32_t printWPL(char *buf, size_t bufsz, const meshtastic_Position &pos, const
                             (abs(geoCoord.getLatitude()) - geoCoord.getDMSLatDeg() * 1e+7) * 6e-6, geoCoord.getDMSLatCP(),
                             geoCoord.getDMSLonDeg(), (abs(geoCoord.getLongitude()) - geoCoord.getDMSLonDeg() * 1e+7) * 6e-6,
                             geoCoord.getDMSLonCP(), name);
+    // snprintf() returns the length that *would* have been written, which can exceed bufsz if
+    // truncated - clamp before using it as an index/size, or the checksum loop reads out of
+    // bounds and bufsz-len underflows into a huge size for the next snprintf().
+    if (len >= bufsz)
+        len = bufsz > 0 ? bufsz - 1 : 0;
+
     uint32_t chk = 0;
     for (uint32_t i = 1; i < len; i++) {
         chk ^= buf[i];
     }
     len += snprintf(buf + len, bufsz - len, "*%02X\r\n", chk);
+    if (len >= bufsz)
+        len = bufsz > 0 ? bufsz - 1 : 0;
     return len;
 }
 /* -------------------------------------------
@@ -91,11 +107,19 @@ uint32_t printGGA(char *buf, size_t bufsz, const meshtastic_Position &pos)
         (abs(geoCoord.getLongitude()) - geoCoord.getDMSLonDeg() * 1e+7) * 6e-6, geoCoord.getDMSLonCP(), pos.fix_quality,
         pos.sats_in_view, pos.HDOP, geoCoord.getAltitude(), 'M', pos.altitude_geoidal_separation, 'M', 0, 0);
 
+    // snprintf() returns the length that *would* have been written, which can exceed bufsz if
+    // truncated - clamp before using it as an index/size, or the checksum loop reads out of
+    // bounds and bufsz-len underflows into a huge size for the next snprintf().
+    if (len >= bufsz)
+        len = bufsz > 0 ? bufsz - 1 : 0;
+
     uint32_t chk = 0;
     for (uint32_t i = 1; i < len; i++) {
         chk ^= buf[i];
     }
     len += snprintf(buf + len, bufsz - len, "*%02X\r\n", chk);
+    if (len >= bufsz)
+        len = bufsz > 0 ? bufsz - 1 : 0;
     return len;
 }
 

src/modules/DropzoneModule.cpp

@@ -32,13 +32,13 @@ ProcessMessage DropzoneModule::handleReceived(const meshtastic_MeshPacket &mp)
     auto &p = mp.decoded;
     char matchCompare[54];
     auto incomingMessage = reinterpret_cast<const char *>(p.payload.bytes);
-    sprintf(matchCompare, "%s conditions", owner.short_name);
+    snprintf(matchCompare, sizeof(matchCompare), "%s conditions", owner.short_name);
     if (strncasecmp(incomingMessage, matchCompare, strlen(matchCompare)) == 0) {
         LOG_DEBUG("Received dropzone conditions request");
         startSendConditions = millis();
     }
 
-    sprintf(matchCompare, "%s conditions", owner.long_name);
+    snprintf(matchCompare, sizeof(matchCompare), "%s conditions", owner.long_name);
     if (strncasecmp(incomingMessage, matchCompare, strlen(matchCompare)) == 0) {
         LOG_DEBUG("Received dropzone conditions request");
         startSendConditions = millis();
@@ -79,11 +79,11 @@ meshtastic_MeshPacket *DropzoneModule::sendConditions()
         auto windDirection = telemetry.variant.environment_metrics.wind_direction;
         auto temp = telemetry.variant.environment_metrics.temperature;
         auto baro = UnitConversions::HectoPascalToInchesOfMercury(telemetry.variant.environment_metrics.barometric_pressure);
-        sprintf(replyStr, "%s @ %02d:%02d:%02dz\nWind %.2f kts @ %d°\nBaro %.2f inHg %.2f°C", dropzoneStatus, hour, min, sec,
-                windSpeed, windDirection, baro, temp);
+        snprintf(replyStr, sizeof(replyStr), "%s @ %02d:%02d:%02dz\nWind %.2f kts @ %d°\nBaro %.2f inHg %.2f°C", dropzoneStatus,
+                 hour, min, sec, windSpeed, windDirection, baro, temp);
     } else {
         LOG_ERROR("No sensor found");
-        sprintf(replyStr, "%s @ %02d:%02d:%02d\nNo sensor found", dropzoneStatus, hour, min, sec);
+        snprintf(replyStr, sizeof(replyStr), "%s @ %02d:%02d:%02d\nNo sensor found", dropzoneStatus, hour, min, sec);
     }
     LOG_DEBUG("Conditions reply: %s", replyStr);
     reply->decoded.payload.size = strlen(replyStr); // You must specify how many bytes are in the reply

src/platform/stm32wl/STM32_LittleFS.cpp

@@ -272,8 +272,8 @@ const char *dbg_strerr_lfs(int32_t err)
         return "LFS_ERR_NOMEM";
 
     default:
-        static char errcode[10];
-        sprintf(errcode, "%ld", err);
+        static char errcode[13]; // "-2147483648\0" (INT32_MIN) is the longest possible value
+        snprintf(errcode, sizeof(errcode), "%ld", err);
         return errcode;
     }