Repository navigation
fix(core): packet parsers test the length before reading header fields (0010 finding 1) - #37
Merged
Merged
Conversation
…the board description Inventory of the radio blocks in the 89 target.cpp (tools/refactoring/ variant_target_globals.py), why objects cannot be weak, the opt-in macro RADIOLIB_TARGET_GLOBALS in target.h, the split into D1 (31 nRF52) and D2 (22 ESP32 with the shared radio_init()), rejected alternatives and the open upstream pull requests that touch the same lines.
…riants A new host suite (native_variant_target_globals) builds the shared variant code against the target.h of a variant with the real AutoDiscoverRTCClock, with the radio block of the 31 nRF52 variants of cut C copied verbatim: the radio is built from a Module on the board pins in SX126x order with SPI passed, radio_driver wraps it and the board, rtc_clock falls back to and ticks fallback_clock, and the shared radio_init() starts that radio. The variant stand-ins move into variant_stubs.h and gain a Module stand-in.
… variants RadioLibTarget.cpp defines radio, radio_driver, fallback_clock and rtc_clock for the variants whose target.h defines the new opt-in macro RADIOLIB_TARGET_GLOBALS, built from the P_LORA_* pins: on nRF52 the radio is a Module on SPI, the fallback clock a VolatileRTCClock. Not weak: a weak object would still be constructed into a variant's own one. Other platforms and the SX1276 pin order stop the build with an #error until described. The 31 nRF52 variants of cut C delete their block from target.cpp (deletions only) and add the macro line to target.h (additions only). The host suite now runs its cases against the shared globals.
…riants A host suite in four environments, one per board description that occurs among the 22 ESP32 variants of cut D2 (SX1262 on an own bus on the default host, without P_LORA_SCLK, SX1276, own bus on another host), with their blocks and radio_init() bodies copied verbatim: the Module's pins in the chip's order, its bus (own, on which host, or none), what radio_driver wraps, the fallback clock, and that radio_init() starts the fallback clock, the RTC and then the radio on its bus, passing the result through. The SPI stand-in gets bus numbers, the radio stand-in tells the module's bus apart.
…r 22 ESP32 variants RadioLibTarget.cpp describes the radio globals for ESP32 as well: the pins in the chip's order (SX1276: DIO0, RESET, DIO1), an own SPIClass where the board names P_LORA_SCLK, on P_LORA_SPI_HOST if set, otherwise no bus so that RadioLib starts its default one, and an ESP32RTCClock as fallback clock. For those variants with an SX126x or SX1276 it adds a weak radio_init(): start the fallback clock and the RTC, then the radio on its bus. The 22 ESP32 variants left by cut C delete their block and radio_init() from target.cpp (deletions only) and add the opt-in, and five of them P_LORA_SPI_HOST (FSPI, or 0 on the C6), to target.h (additions only).
…radio_init() of 22 ESP32 variants
…their source lines
…their source lines
… (step 11, cut D1)
…board description (step 11, cut D2)
test_packet_parse (native_core) runs Dispatcher::tryParsePacket() and Packet::readFrom() on a buffer with an unreadable page behind len, and compares both, and the radio receive path, against the code as it is. A companion test sends short SEND_RAW_PACKET frames of every route type. The two guarded cases fail on this commit: both parsers read the header fields before they test the length.
A child that reads the guard page now exits with 3 instead of crashing, so the system crash reporter is not started for every failing case, and the death tests use the fork style: 2 s instead of 30 s.
Dispatcher::tryParsePacket() read the transport codes and the path length byte, and Packet::readFrom() also the whole path, before comparing with len. Both now test the length first. Every packet is accepted or refused as before, and an accepted packet has the same fields (docs/fixes/packet-parse-len.md).
Notes the fix, the callers it is reached from, the host tests, mutations, the ASan probe before and after, native runs on macOS and Linux and the builds per platform. Open finding 1 of note 0010 points to it.
marcelverdult
added a commit
that referenced
this pull request
Oct 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CI only, not for merge into integration. Based on
integrationat07fa9c5c, so the diff againstdevalso shows the integration commits. This PR's own commits are the last four, from783e89d2.Open finding 1 of note 0010:
Dispatcher::tryParsePacket()read the transport codes and the path length byte before testinglen, andPacket::readFrom()also copied the whole path first. Both now test the length before each read. No packet changes its outcome: every one is accepted or refused as before, accepted ones with the same fields. Note: docs/fixes/packet-parse-len.md.Evidence:
test/test_packet_parseinnative_core, plusCommandTest.SendRawPacketShortFramesOfEveryRouteType): input at the end of a page with aPROT_NONEpage behind it, every route type,len0 to 72; on the old code 288 cases fail exactly where predicted (tryParsePacket route 0/3 len 0..5, route 1/2 len 0..1; readFrom up to len 69/65), after the fix none. Reference comparison against the old code kept verbatim in the test (16 version/route pairs, all 256 path length bytes,len0..80 and 180..256, two stale fills) for both parsers and for the radio receive path: equal before and after. Frame sweep golden untouched.companion_asan_probe.patch,PROBE_SKIP=1): thecmdSendRawPacket/tryParsePacketline (code 65, lens 4..6) is gone; the CLI finding 2 stays unchanged.python:3.12-slim+ pip platformio).