From d9195944dff29a94b58f2e6c1a884e2ce111462f Mon Sep 17 00:00:00 2001 From: nightjoker7 <47129685+nightjoker7@users.noreply.github.com> Date: Thu, 23 Apr 2026 18:20:07 -0500 Subject: [PATCH] PositionModule::sendLostAndFoundText: use stack buffer, eliminate heap alloc (#10251) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * PositionModule::sendLostAndFoundText: use stack buffer, eliminate heap alloc The lost-and-found message was built with an unnecessary heap allocation: char *message = new char[60]; sprintf(message, "..."...); ... delete[] message; Two problems: 1. **Buffer too small.** The format string expands with two %f (IEEE 754 doubles), which `sprintf` prints with full precision — easily 15+ digits each plus separators — so the actual rendered string can run 40-50 characters before even considering the emoji (4 UTF-8 bytes) and the embedded BEL. A pathological lat/lon can overflow 60 bytes and corrupt heap metadata. Unbounded `sprintf` with no size check. 2. **Heap churn in a GPS callback.** This function is called from the position-update path which is already heap-sensitive. An infrequent 60-byte transient alloc isn't catastrophic, but stack is trivially available here and removes the failure mode entirely. Fix: replace with a 128-byte stack buffer and `snprintf` bounded by `sizeof(message)`. Drop the matching `delete[]` since there's nothing to delete. Behavior is identical on the happy path; the overflow case now truncates safely instead of scribbling over heap. * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * PositionModule.cpp: add trailing newline for clang-format * Address Copilot review: cleaner snprintf size handling Review feedback from @Copilot on PR #10251: the ternary-plus-static-cast form mixed signed/unsigned types (int written vs. pb_size_t payload.size vs. size_t sizeof(message)) and was harder to read than necessary. Cleaner form: const size_t msg_len = std::min(static_cast(written), sizeof(message) - 1); p->decoded.payload.size = msg_len; Same behaviour (clamp to buffer-minus-NUL) with one explicit cast and a size_t variable that names the meaning. Handles the encoding-error path (written < 0) separately so no bad values leak into payload.size. * Trunk --------- Co-authored-by: Ben Meadors Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/modules/PositionModule.cpp | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/modules/PositionModule.cpp b/src/modules/PositionModule.cpp index 0378d01e7..ac81e9c57 100644 --- a/src/modules/PositionModule.cpp +++ b/src/modules/PositionModule.cpp @@ -492,15 +492,24 @@ void PositionModule::sendLostAndFoundText() { meshtastic_MeshPacket *p = allocDataPacket(); p->to = NODENUM_BROADCAST; - char *message = new char[60]; - sprintf(message, "ðŸšĻI'm lost! Lat / Lon: %f, %f\a", (lastGpsLatitude * 1e-7), (lastGpsLongitude * 1e-7)); + char message[128]; + int written = snprintf(message, sizeof(message), "ðŸšĻI'm lost! Lat / Lon: %f, %f\a", (lastGpsLatitude * 1e-7), + (lastGpsLongitude * 1e-7)); p->decoded.portnum = meshtastic_PortNum_TEXT_MESSAGE_APP; p->want_ack = false; - p->decoded.payload.size = strlen(message); - memcpy(p->decoded.payload.bytes, message, p->decoded.payload.size); + if (written < 0) { + // snprintf encoding error — send an empty payload rather than uninitialized bytes. + p->decoded.payload.size = 0; + } else { + // Clamp to buffer capacity (snprintf returns "would-have-written" which can exceed the buffer). + const size_t msg_len = std::min(static_cast(written), sizeof(message) - 1); + p->decoded.payload.size = msg_len; + if (msg_len > 0) { + memcpy(p->decoded.payload.bytes, message, msg_len); + } + } service->sendToMesh(p, RX_SRC_LOCAL, true); - delete[] message; } // Helper: return imprecise (truncated + centered) lat/lon as int32 using current precision @@ -580,4 +589,4 @@ void PositionModule::handleNewPosition() } } -#endif \ No newline at end of file +#endif