diff --git a/src/mesh/RadioInterface.cpp b/src/mesh/RadioInterface.cpp index 5db23ab544..c0212f4ded 100644 --- a/src/mesh/RadioInterface.cpp +++ b/src/mesh/RadioInterface.cpp @@ -1518,7 +1518,13 @@ size_t RadioInterface::beginSending(meshtastic_MeshPacket *p) // if the sender nodenum is zero, that means uninitialized assert(radioBuffer.header.from); - assert(p->encrypted.size <= sizeof(radioBuffer.payload)); + // Runtime packet payload size bounds check against radioBuffer to prevent overflow in memcpy() + if (static_cast(p->encrypted.size) > sizeof(radioBuffer.payload)) { + LOG_ERROR("Packet payload size %u exceeds radioBuffer capacity %u", static_cast(p->encrypted.size), + static_cast(sizeof(radioBuffer.payload))); + packetPool.release(p); + return 0; + } memcpy(radioBuffer.payload, p->encrypted.bytes, p->encrypted.size); sendingPacket = p; diff --git a/src/mesh/RadioLibInterface.cpp b/src/mesh/RadioLibInterface.cpp index 195a5738a0..3018a34dd3 100644 --- a/src/mesh/RadioLibInterface.cpp +++ b/src/mesh/RadioLibInterface.cpp @@ -597,12 +597,13 @@ void RadioLibInterface::completeSending() printPacket("Completed sending", p); #if !MESHTASTIC_EXCLUDE_BEACON MeshBeaconModule::clearTargetRadioSettings(p); - MeshBeaconModule::reconfigureForBeaconTX(this, nullptr); #endif - // We are done sending that packet, release it packetPool.release(p); } +#if !MESHTASTIC_EXCLUDE_BEACON + MeshBeaconModule::reconfigureForBeaconTX(this, nullptr); +#endif } void RadioLibInterface::handleReceiveInterrupt() @@ -782,7 +783,18 @@ bool RadioLibInterface::startSend(meshtastic_MeshPacket *txp) } else { configHardwareForSend(); // must be after setStandby +#if !MESHTASTIC_EXCLUDE_BEACON + MeshBeaconModule::clearTargetRadioSettings(txp); +#endif size_t numbytes = beginSending(txp); + if (numbytes == 0) { + if (!sendingPacket) { + completeSending(); + powerMon->clearState(meshtastic_PowerMon_State_Lora_TXOn); + startReceive(); + } + return false; + } int res = iface->startTransmit((uint8_t *)&radioBuffer, numbytes); if (res != RADIOLIB_ERR_NONE) { diff --git a/src/modules/MeshBeaconModule.cpp b/src/modules/MeshBeaconModule.cpp index a747621838..9982f8d157 100644 --- a/src/modules/MeshBeaconModule.cpp +++ b/src/modules/MeshBeaconModule.cpp @@ -286,7 +286,10 @@ void MeshBeaconBroadcastModule::sendBeaconPacket(meshtastic_MeshPacket *p, mesht const bool cryptoOverride = has_channel && overrideChannel && (overrideChannel->name[0] != '\0' || overrideChannel->psk.size > 0); if (!cryptoOverride) { - router->send(p); + if (router->send(p) == ERRNO_SHOULD_RELEASE) { + MeshBeaconModule::clearTargetRadioSettings(p); + packetPool.release(p); + } return; } @@ -300,7 +303,10 @@ void MeshBeaconBroadcastModule::sendBeaconPacket(meshtastic_MeshPacket *p, mesht primary.settings = beaconChannelSettings(saved, targetPreset, overrideChannel); channels.fixupChannel(channels.getPrimaryIndex()); - router->send(p); // encrypts with the beacon channel's key and stamps its hash + if (router->send(p) == ERRNO_SHOULD_RELEASE) { // encrypts with the beacon channel's key and stamps its hash + MeshBeaconModule::clearTargetRadioSettings(p); + packetPool.release(p); + } primary.settings = saved; channels.fixupChannel(channels.getPrimaryIndex()); diff --git a/test/test_radio/test_main.cpp b/test/test_radio/test_main.cpp index 87f3f37245..f8a701f2a3 100644 --- a/test/test_radio/test_main.cpp +++ b/test/test_radio/test_main.cpp @@ -60,6 +60,10 @@ class TestableRadioInterface : public RadioInterface uint8_t getSf() const { return sf; } float getBw() const { return bw; } + size_t beginSendingPublic(meshtastic_MeshPacket *p) { return beginSending(p); } + meshtastic_MeshPacket *getSendingPacket() const { return sendingPacket; } + size_t getRadioBufferPayloadCapacity() const { return sizeof(radioBuffer.payload); } + // Override reconfigure to call the base which invokes applyModemConfig() bool reconfigure() override { return RadioInterface::reconfigure(); } @@ -413,6 +417,30 @@ static void test_regionPresetMap_unsetCarriesUserprefsIntent() #endif } +static void test_beginSending_oversizedPayloadAbortsSafely() +{ + meshtastic_MeshPacket *p = packetPool.allocZeroed(); + TEST_ASSERT_NOT_NULL(p); + p->from = 0x12345678; + p->to = 0x87654321; + p->id = 0x10203040; + p->which_payload_variant = meshtastic_MeshPacket_encrypted_tag; + + // Set encrypted size larger than sizeof(radioBuffer.payload) (which is 256 - sizeof(PacketHeader)) + p->encrypted.size = testRadio->getRadioBufferPayloadCapacity() + 10; + + size_t result = testRadio->beginSendingPublic(p); + + TEST_ASSERT_EQUAL_UINT(0, result); + TEST_ASSERT_NULL(testRadio->getSendingPacket()); + + // Verify rejected packet was released to packetPool and its slot is reusable + meshtastic_MeshPacket *reallocated = packetPool.allocZeroed(); + TEST_ASSERT_NOT_NULL(reallocated); + TEST_ASSERT_EQUAL_PTR(p, reallocated); + packetPool.release(reallocated); +} + void setUp(void) { mockMeshService = new MockMeshService(); @@ -463,6 +491,7 @@ void setup() RUN_TEST(test_regionPresetMap_coversAllRegionsWithinBounds); RUN_TEST(test_regionPresetMap_matchesRegionTable); RUN_TEST(test_regionPresetMap_unsetCarriesUserprefsIntent); + RUN_TEST(test_beginSending_oversizedPayloadAbortsSafely); exit(UNITY_END()); }