From 8b7c5fafb423de1fbde5170c7c9d27cecab5d200 Mon Sep 17 00:00:00 2001 From: mikecarper Date: Tue, 25 Aug 2026 10:27:04 -0700 Subject: [PATCH] Validate companion repeater status responses --- examples/companion_radio/MyMesh.cpp | 20 +++++++----- src/helpers/CompanionStatusResponse.h | 25 +++++++++++++++ test/README.md | 1 + .../test_companion_status_response.cpp | 31 +++++++++++++++++++ 4 files changed, 70 insertions(+), 7 deletions(-) create mode 100644 src/helpers/CompanionStatusResponse.h create mode 100644 test/test_companion_status_response/test_companion_status_response.cpp diff --git a/examples/companion_radio/MyMesh.cpp b/examples/companion_radio/MyMesh.cpp index eeaa85a90..b7a44c370 100644 --- a/examples/companion_radio/MyMesh.cpp +++ b/examples/companion_radio/MyMesh.cpp @@ -2,6 +2,7 @@ #include // needed for PlatformIO #include +#include #include #include "helpers/radiolib/RXPowerSaving.h" #include "helpers/radiolib/RxBoostedGainDefaults.h" @@ -1163,13 +1164,19 @@ void MyMesh::onContactResponse(const ContactInfo &contact, const uint8_t *data, clearTerminalLogin(); } #endif - } else if (len > 4 && // check for status response - pending_status && - memcmp(&pending_status, contact.id.pub_key, 4) == 0 // legacy matching scheme - // FUTURE: tag == pending_status - ) { + } else if (mesh::companionStatusTagMatches(pending_status, tag)) { pending_status = 0; + // Do not expose a truncated or unrelated response as repeater statistics. + // The app parses at least 48 bytes and otherwise throws a RangeError. + if (!mesh::companionStatusResponseIsLongEnough(len)) { + MESH_DEBUG_PRINTLN( + "onContactResponse(), short status response: len=%u, expected>=%u", + (unsigned)len, + (unsigned)mesh::COMPANION_MIN_STATUS_RESPONSE_SIZE); + return; + } + int i = 0; out_frame[i++] = PUSH_CODE_STATUS_RESPONSE; out_frame[i++] = 0; // reserved @@ -3366,8 +3373,7 @@ void MyMesh::handleCmdFrame(size_t len) { writeErrFrame(ERR_CODE_TABLE_FULL); } else { clearPendingReqs(); - // FUTURE: pending_status = tag; // match this in onContactResponse() - memcpy(&pending_status, recipient->id.pub_key, 4); // legacy matching scheme + pending_status = tag; // match the reflected tag in onContactResponse() out_frame[0] = RESP_CODE_SENT; out_frame[1] = (result == MSG_SEND_SENT_FLOOD) ? 1 : 0; memcpy(&out_frame[2], &tag, 4); diff --git a/src/helpers/CompanionStatusResponse.h b/src/helpers/CompanionStatusResponse.h new file mode 100644 index 000000000..d8669e27a --- /dev/null +++ b/src/helpers/CompanionStatusResponse.h @@ -0,0 +1,25 @@ +#pragma once + +#include +#include + +namespace mesh { + +// A repeater/room-server status response starts with the reflected four-byte +// request tag. The app currently parses 48 bytes of status fields after that +// tag; newer firmware may append additional counters. +static constexpr size_t COMPANION_STATUS_RESPONSE_TAG_SIZE = 4; +static constexpr size_t COMPANION_MIN_STATUS_DATA_SIZE = 48; +static constexpr size_t COMPANION_MIN_STATUS_RESPONSE_SIZE = + COMPANION_STATUS_RESPONSE_TAG_SIZE + COMPANION_MIN_STATUS_DATA_SIZE; + +inline bool companionStatusTagMatches(uint32_t pending_tag, + uint32_t response_tag) { + return pending_tag != 0 && response_tag == pending_tag; +} + +inline bool companionStatusResponseIsLongEnough(size_t response_len) { + return response_len >= COMPANION_MIN_STATUS_RESPONSE_SIZE; +} + +} // namespace mesh diff --git a/test/README.md b/test/README.md index ab78943c0..8afcbd06b 100644 --- a/test/README.md +++ b/test/README.md @@ -48,6 +48,7 @@ does not reflect the GoogleTest count -- run the built binary directly | `test_identity_generation` | `src/helpers/IdentityGeneration.h` | reserved-prefix rejection; bounded retries; final provisioned attempt; fail-closed exhaustion | | `test_remote_cli_reply_cache` | `src/helpers/RemoteCliReplyCache.h`, `src/helpers/RemoteCliRequest.h`, `src/helpers/RemoteCliTimeout.h` | authenticated logical-request matching; bounded recent-reply history; backward-compatible retry identity; 300% response timeout; empty-response completion; on-air truncation and clearing | | `test_companion_frame_queue` | `src/helpers/CompanionFrameQueue.h` | response/required/best-effort classification; reserved capacity; stable priority; safe eviction; message-waiting coalescing | +| `test_companion_status_response` | `src/helpers/CompanionStatusResponse.h` | request-tag correlation and minimum status-response length, including rejection of the three-entry ACL payload that previously masqueraded as status | | `test_serial_mode_switch` | `src/helpers/ArduinoSerialInterface.cpp`, `src/helpers/MultiSerialInterface.h` | terminal/seeder control-sequence recognition and passthrough ownership; queued/atomic USB output under backpressure and short writes; partial-frame busy state; requester-affine replies, locked contact streams, and Bluetooth-only pairing routing | | `test_ble_tx_stall_watchdog` | `src/helpers/BleTxStallWatchdog.h` | exact BLE fragment progress; blocked-reply timeout; rollover-safe elapsed time; disconnect recovery retry and completion | | `test_atomic_file_writer` | `src/helpers/AtomicFileWriter.h` | verified temporary-file commit; short-write, readback, validation, and rename failures; preservation of the live file and stale-temp cleanup | diff --git a/test/test_companion_status_response/test_companion_status_response.cpp b/test/test_companion_status_response/test_companion_status_response.cpp new file mode 100644 index 000000000..aa0b54e4c --- /dev/null +++ b/test/test_companion_status_response/test_companion_status_response.cpp @@ -0,0 +1,31 @@ +#include + +#include + +TEST(CompanionStatusResponse, RequiresTheExactPendingRequestTag) { + EXPECT_FALSE(mesh::companionStatusTagMatches(0, 0)); + EXPECT_FALSE(mesh::companionStatusTagMatches(0, 0x12345678)); + EXPECT_FALSE(mesh::companionStatusTagMatches(0x12345678, 0x87654321)); + EXPECT_TRUE(mesh::companionStatusTagMatches(0x12345678, 0x12345678)); +} + +TEST(CompanionStatusResponse, RejectsThreeEntryAclPayloadAsStatus) { + // An ACL response is tag + seven bytes per entry. Three entries produced + // the 21-byte payload that the app incorrectly attempted to parse as stats. + constexpr size_t three_entry_acl_response = 4 + (3 * 7); + EXPECT_FALSE( + mesh::companionStatusResponseIsLongEnough(three_entry_acl_response)); +} + +TEST(CompanionStatusResponse, AcceptsLegacyMinimumAndCurrentStatusSizes) { + EXPECT_FALSE(mesh::companionStatusResponseIsLongEnough( + mesh::COMPANION_MIN_STATUS_RESPONSE_SIZE - 1)); + EXPECT_TRUE(mesh::companionStatusResponseIsLongEnough( + mesh::COMPANION_MIN_STATUS_RESPONSE_SIZE)); + EXPECT_TRUE(mesh::companionStatusResponseIsLongEnough(4 + 56)); +} + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +}