diff --git a/src/RedirectablePrint.cpp b/src/RedirectablePrint.cpp index e53806ac77a..f6641fdbdce 100644 --- a/src/RedirectablePrint.cpp +++ b/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); diff --git a/src/gps/NMEAWPL.cpp b/src/gps/NMEAWPL.cpp index 5259427e635..72ed6e5cd08 100644 --- a/src/gps/NMEAWPL.cpp +++ b/src/gps/NMEAWPL.cpp @@ -2,8 +2,27 @@ #include "NMEAWPL.h" #include "GeoCoord.h" #include "gps/RTC.h" +#include #include +static uint32_t nmeaClamp(uint32_t len, size_t bufsz) +{ + if (len >= bufsz) + return bufsz > 0 ? (uint32_t)(bufsz - 1) : 0; + return len; +} + +static uint32_t nmeaChecksum(const char *buf) +{ + uint32_t chk = 0; + const char *c = strchr(buf, '$'); + if (c) { + for (c++; *c && *c != '*'; c++) + chk ^= (uint8_t)*c; + } + return chk; +} + /* ------------------------------------------- * 1 2 3 4 5 6 * | | | | | | @@ -21,33 +40,35 @@ uint32_t printWPL(char *buf, size_t bufsz, const meshtastic_PositionLite &pos, const char *name, bool isCaltopoMode) { + if (bufsz == 0) + return 0; + GeoCoord geoCoord(pos.latitude_i, pos.longitude_i, pos.altitude); char type = isCaltopoMode ? 'P' : 'N'; uint32_t len = snprintf(buf, bufsz, "\r\n$G%cWPL,%02d%07.4f,%c,%03d%07.4f,%c,%s", type, geoCoord.getDMSLatDeg(), (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); - 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); + len = nmeaClamp(len, bufsz); + uint32_t chk = nmeaChecksum(buf); + len = nmeaClamp(len + snprintf(buf + len, bufsz - len, "*%02X\r\n", chk), bufsz); return len; } uint32_t printWPL(char *buf, size_t bufsz, const meshtastic_Position &pos, const char *name, bool isCaltopoMode) { + if (bufsz == 0) + return 0; + GeoCoord geoCoord(pos.latitude_i, pos.longitude_i, pos.altitude); char type = isCaltopoMode ? 'P' : 'N'; uint32_t len = snprintf(buf, bufsz, "$G%cWPL,%02d%07.4f,%c,%03d%07.4f,%c,%s", type, geoCoord.getDMSLatDeg(), (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); - 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); + len = nmeaClamp(len, bufsz); + uint32_t chk = nmeaChecksum(buf); + len = nmeaClamp(len + snprintf(buf + len, bufsz - len, "*%02X\r\n", chk), bufsz); return len; } /* ------------------------------------------- @@ -74,6 +95,9 @@ uint32_t printWPL(char *buf, size_t bufsz, const meshtastic_Position &pos, const uint32_t printGGA(char *buf, size_t bufsz, const meshtastic_Position &pos) { + if (bufsz == 0) + return 0; + GeoCoord geoCoord(pos.latitude_i, pos.longitude_i, pos.altitude); time_t timestamp = pos.timestamp; @@ -91,11 +115,9 @@ 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); - 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); + len = nmeaClamp(len, bufsz); + uint32_t chk = nmeaChecksum(buf); + len = nmeaClamp(len + snprintf(buf + len, bufsz - len, "*%02X\r\n", chk), bufsz); return len; } diff --git a/src/modules/DropzoneModule.cpp b/src/modules/DropzoneModule.cpp index 4658cb5b6ec..16bd8384969 100644 --- a/src/modules/DropzoneModule.cpp +++ b/src/modules/DropzoneModule.cpp @@ -82,11 +82,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 diff --git a/src/platform/stm32wl/STM32_LittleFS.cpp b/src/platform/stm32wl/STM32_LittleFS.cpp index 97e79e61e7e..6d804d742b6 100644 --- a/src/platform/stm32wl/STM32_LittleFS.cpp +++ b/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]; + snprintf(errcode, sizeof(errcode), "%ld", (long)err); return errcode; } diff --git a/test/native-suite-count b/test/native-suite-count index d81cc0710eb..920a1396648 100644 --- a/test/native-suite-count +++ b/test/native-suite-count @@ -1 +1 @@ -42 +43 diff --git a/test/test_nmea_wpl/test_main.cpp b/test/test_nmea_wpl/test_main.cpp new file mode 100644 index 00000000000..4cef767200e --- /dev/null +++ b/test/test_nmea_wpl/test_main.cpp @@ -0,0 +1,158 @@ +#include "GeoCoord.h" +#include "NMEAWPL.h" +#include "TestUtil.h" +#include "mesh-pb-constants.h" +#include +#include +#include +#include +#include + +void setUp(void) {} +void tearDown(void) {} + +static meshtastic_PositionLite makePositionLite() +{ + meshtastic_PositionLite pos = meshtastic_PositionLite_init_default; + pos.latitude_i = 472852133; + pos.longitude_i = 85652500; + pos.altitude = 400; + pos.time = 42; + return pos; +} + +static meshtastic_Position makePosition() +{ + meshtastic_Position pos = meshtastic_Position_init_default; + pos.has_latitude_i = true; + pos.latitude_i = 472852133; + pos.has_longitude_i = true; + pos.longitude_i = 85652500; + pos.has_altitude = true; + pos.altitude = 400; + pos.timestamp = 43; + return pos; +} + +static uint32_t expectedChecksum(const char *sentence) +{ + uint32_t chk = 0; + const char *c = strchr(sentence, '$'); + TEST_ASSERT_NOT_NULL(c); + for (c++; *c && *c != '*'; c++) + chk ^= (uint8_t)*c; + return chk; +} + +static uint32_t emittedChecksum(const char *buf) +{ + const char *star = strrchr(buf, '*'); + TEST_ASSERT_NOT_NULL(star); + TEST_ASSERT_TRUE(isxdigit((unsigned char)star[1])); + TEST_ASSERT_TRUE(isxdigit((unsigned char)star[2])); + TEST_ASSERT_TRUE(star[3] == '\r' || star[3] == '\0'); + unsigned parsed = 0; + TEST_ASSERT_EQUAL_INT(1, sscanf(star + 1, "%02X", &parsed)); + return parsed; +} + +static void assertChecksumMatchesBody(const char *buf) +{ + TEST_ASSERT_EQUAL_UINT32(expectedChecksum(buf), emittedChecksum(buf)); +} + +void test_wpl_lite_checksum_skips_leading_crlf(void) +{ + char buf[128]; + meshtastic_PositionLite pos = makePositionLite(); + uint32_t len = printWPL(buf, sizeof(buf), pos, "Test", false); + + TEST_ASSERT_TRUE(len < sizeof(buf)); + TEST_ASSERT_EQUAL_CHAR('\r', buf[0]); + TEST_ASSERT_EQUAL_CHAR('\n', buf[1]); + TEST_ASSERT_EQUAL_CHAR('$', buf[2]); + assertChecksumMatchesBody(buf); +} + +void test_wpl_position_checksum(void) +{ + char buf[128]; + meshtastic_Position pos = makePosition(); + uint32_t len = printWPL(buf, sizeof(buf), pos, "Test", false); + + TEST_ASSERT_TRUE(len < sizeof(buf)); + TEST_ASSERT_EQUAL_CHAR('$', buf[0]); + assertChecksumMatchesBody(buf); +} + +void test_gga_checksum(void) +{ + char buf[160]; + meshtastic_Position pos = makePosition(); + uint32_t len = printGGA(buf, sizeof(buf), pos); + + TEST_ASSERT_TRUE(len < sizeof(buf)); + TEST_ASSERT_EQUAL_CHAR('$', buf[0]); + assertChecksumMatchesBody(buf); +} + +void test_crlf_prefix_does_not_change_checksum(void) +{ + const uint32_t fixtureChecksum = 0x69; + char withPrefix[128]; + char withoutPrefix[128]; + meshtastic_PositionLite lite = makePositionLite(); + meshtastic_Position pos = makePosition(); + + printWPL(withPrefix, sizeof(withPrefix), lite, "Test", false); + printWPL(withoutPrefix, sizeof(withoutPrefix), pos, "Test", false); + + TEST_ASSERT_EQUAL_UINT32(fixtureChecksum, emittedChecksum(withPrefix)); + TEST_ASSERT_EQUAL_UINT32(fixtureChecksum, emittedChecksum(withoutPrefix)); +} + +void test_zero_sized_buffer_writes_nothing(void) +{ + char buf[64]; + memset(buf, 'A', sizeof(buf)); + meshtastic_PositionLite lite = makePositionLite(); + meshtastic_Position pos = makePosition(); + + TEST_ASSERT_EQUAL_UINT32(0, printWPL(buf, 0, lite, "Test", false)); + TEST_ASSERT_EQUAL_UINT32(0, printWPL(buf, 0, pos, "Test", false)); + TEST_ASSERT_EQUAL_UINT32(0, printGGA(buf, 0, pos)); + + for (size_t i = 0; i < sizeof(buf); i++) + TEST_ASSERT_EQUAL_CHAR('A', buf[i]); +} + +void test_truncated_buffers_stay_in_bounds(void) +{ + const size_t sizes[] = {1, 2, 8, 20, 40}; + meshtastic_PositionLite lite = makePositionLite(); + + for (size_t s = 0; s < sizeof(sizes) / sizeof(sizes[0]); s++) { + char buf[128]; + memset(buf, 0x7E, sizeof(buf)); + uint32_t len = printWPL(buf, sizes[s], lite, "Test", false); + + TEST_ASSERT_TRUE(len < sizes[s]); + for (size_t i = sizes[s]; i < sizeof(buf); i++) + TEST_ASSERT_EQUAL_HEX8(0x7E, (uint8_t)buf[i]); + } +} + +void setup() +{ + initializeTestEnvironment(); + UNITY_BEGIN(); + RUN_TEST(test_wpl_lite_checksum_skips_leading_crlf); + RUN_TEST(test_wpl_position_checksum); + RUN_TEST(test_gga_checksum); + RUN_TEST(test_crlf_prefix_does_not_change_checksum); + RUN_TEST(test_zero_sized_buffer_writes_nothing); + RUN_TEST(test_truncated_buffers_stay_in_bounds); + exit(UNITY_END()); +} + +void loop() {}