From 988b438ec38c8878ce11b0f1852f455af2d6bcee Mon Sep 17 00:00:00 2001
From: liquidraver <504870+liquidraver@users.noreply.github.com>
Date: Wed, 20 May 2026 11:57:37 +0200
Subject: [PATCH] refactor(companion): harden telemetry buffer sizing and
 custom-vars snprintf
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two polish items from BLE audit Phase 2B:

1. CMD_SEND_TELEMETRY_REQ self-response buffer was uint8_t rsp[96]
   with a comment claiming 70 B worst case. Actual worst case at
   POWER_MAX_CHANNELS=4 is 82 B; if the channel cap ever grew the
   buffer would silently overflow. Replaced with a sizeof-style
   expression that tracks POWER_MAX_CHANNELS, plus an 8-byte safety
   pad. No size change today (90 vs. 96) but the upper bound auto-
   tracks any future bump.

2. CMD_GET_CUSTOM_VARS used `dp += snprintf(dp, 20, ...)` which
   advances by the would-be-written length, not bytes actually
   written. Currently safe only because gps_interval is capped
   ≤86400, but if either cap drifted or a new key was added the
   length passed to writeFrame would include uninitialized stack
   bytes between the truncation point and the (over-advanced) dp.
   Now tracks rsp_end, computes remaining per snprintf, and only
   advances dp on real progress.

Both are correctness polish, not exploitable today.
---
 zephcore/app/CompanionMesh.cpp | 36 +++++++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/zephcore/app/CompanionMesh.cpp b/zephcore/app/CompanionMesh.cpp
index bc3cf87..59bbae8 100644
--- a/zephcore/app/CompanionMesh.cpp
+++ b/zephcore/app/CompanionMesh.cpp
@@ -2365,7 +2365,11 @@ bool CompanionMesh::handleProtocolFrame(const uint8_t *data, size_t len)
 			// Self-telemetry request: return battery, GPS, and environment data
 			// Format: Cayenne LPP: [channel][type][data...]
 			// Response: [PUSH_CODE_TELEMETRY_RESPONSE][reserved][6-byte pubkey][telemetry_data]
-			uint8_t rsp[96];  // header(8)+batt(4)+gps(11)+env(11)+pwr(36)=70 worst case
+			// Worst-case size tracks POWER_MAX_CHANNELS so a future bump can't
+			// silently overflow this stack buffer. With current value 4:
+			// header(8) + batt(4) + gps(11) + env(temp4+hum3+press4=11)
+			// + power(POWER_MAX_CHANNELS * 12 = 48) + 8 byte safety pad = 90.
+			uint8_t rsp[8 + 4 + 11 + 11 + (12 * POWER_MAX_CHANNELS) + 8];
 			int i = 0;
 			rsp[i++] = PUSH_CODE_TELEMETRY_RESPONSE;
 			rsp[i++] = 0;  // reserved
@@ -2615,19 +2619,37 @@ bool CompanionMesh::handleProtocolFrame(const uint8_t *data, size_t len)
 		// Format: [PACKET_CUSTOM_VARS][key1:val1,key2:val2,...]
 		uint8_t rsp[64];
 		char *dp = (char *)&rsp[1];
+		char *const rsp_end = (char *)&rsp[sizeof(rsp)];
 		rsp[0] = PACKET_CUSTOM_VARS;
 
-		// GPS settings
+		// snprintf returns the would-be length, NOT bytes actually written.
+		// We must check truncation and only advance dp on real progress —
+		// otherwise dp can outrun the initialized portion of rsp and
+		// writeFrame(rsp, dp-rsp) leaks adjacent stack to the phone.
+
 		bool first = true;
 		if (gps_is_available()) {
-			dp += snprintf(dp, 20, "gps:%d", gps_is_enabled() ? 1 : 0);
-			first = false;
+			size_t remaining = (size_t)(rsp_end - dp);
+			int n = snprintf(dp, remaining, "gps:%d", gps_is_enabled() ? 1 : 0);
+			if (n > 0 && (size_t)n < remaining) {
+				dp += n;
+				first = false;
+			}
 		}
 		uint32_t gps_interval = gps_get_poll_interval_sec();
 		if (gps_interval > 0) {
-			if (!first) *dp++ = ',';
-			dp += snprintf(dp, 20, "gps_interval:%u", gps_interval);
-			first = false;
+			size_t remaining = (size_t)(rsp_end - dp);
+			// Reserve 1 byte for the leading ',' if needed
+			if (!first && remaining > 0) {
+				*dp++ = ',';
+				remaining--;
+			}
+			int n = snprintf(dp, remaining, "gps_interval:%u", (unsigned)gps_interval);
+			if (n > 0 && (size_t)n < remaining) {
+				dp += n;
+			}
+			// If snprintf would have truncated, dp stays put — writeFrame
+			// sends only what we successfully wrote.
 		}
 		// Note: Environment sensors are auto-detected, no settings needed