fix(radio): MeshBeacon heap leak and runtime packet payload size check (#11573)

* Fix for MeshBeacon packet leakage

* fix: add runtime payload size check against radiobuffer

* review fix for PR#11573: clear target radio settings before MeshBeacon packet release

* add unit test for radio buffer capacity check, removing related assert for the test

* review fix for PR#11573: add explicit verifaction against rejected packets

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<size_t>(p->encrypted.size) > sizeof(radioBuffer.payload)) {
+        LOG_ERROR("Packet payload size %u exceeds radioBuffer capacity %u", static_cast<unsigned>(p->encrypted.size),
+                  static_cast<unsigned>(sizeof(radioBuffer.payload)));
+        packetPool.release(p);
+        return 0;
+    }
     memcpy(radioBuffer.payload, p->encrypted.bytes, p->encrypted.size);
 
     sendingPacket = p;

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) {

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());

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());
 }