firmware/docs/audit/REPORT.md

Security Audit Report — donglora firmware v1.5.2

Auditor: Mythos (adversarial security review per MYTHOS.md) Repo: /home/swaits/Code/donglora-org/firmware, HEAD 688938d (chore: release 1.5.2) Phases: Recon (01-recon.md) → Threats (02-threats.md) → Targeted analysis (03-findings-draft.md) → Verification → this report.

---

Executive summary

3 findings total: 0 Critical, 0 High, 0 Medium, 2 Low, 1 Info.

The firmware is a small, tightly-scoped, no_std embedded Rust binary that exposes a SX126x/SX127x radio over a single host transport (USB CDC-ACM or UART). It performs no networking, no filesystem I/O, no shell exec, no template rendering, no SQL/NoSQL/XML, no cryptography, and no deserialization beyond explicit byte slicing. The whole vulnerability landscape MYTHOS targets is out of scope by construction; the realistic attack surface is exactly two paths: (1) inbound frames on the host transport, and (2) inbound LoRa packets on the configured RF parameters.

Both paths were traced end-to-end:

• Host-transport ingress. Every reachable byte goes through a CRC-checked, COBS-framed FrameDecoder → strict typed Command::parse → numeric-bounded validate_lora. Reserved bits, unknown enum values, length mismatches, and tag=0 are all explicitly rejected and turned into ERR(...) responses. No reachable panic, no buffer overrun, no integer overflow, no OOB access was found.
• OTA RX ingress. Payload bytes are bounded to 255 (SX126x register width); copied into a heapless::Vec<u8, 255> and forwarded to the host verbatim. The firmware never parses OTA bytes — there is no interpretation surface for a remote attacker to exploit.

The remaining findings concern disclosure (MAC visible in the USB serial descriptor — by design but worth documenting), supply-chain hardening (git dep not commit-pinned), and reliability on one board variant (esp-println output collides with host UART on lilygo_tbeam classic only). All are Low or Info severity.

Top 3 systemic observations (NOT findings — design choices flagged for transparency):

1. No authentication on host transport. Anyone with kernel-level access to the CDC-ACM/UART device node controls the radio. The product is intentionally a "transparent dongle" — physical attachment is the auth boundary.
2. No RF-regulatory enforcement. Frequency, sync word, and TX power are whatever the host commands, within hardware capability. FCC / CE / ETSI compliance is the user's responsibility.
3. TX skip_cad=1 bypasses listen-before-talk on demand. Documented in PROTOCOL.md; available to any host-side actor.

These are deliberate features of a transparent LoRa bridge; treating them as bugs would be a category error. They are surfaced here so a security reader knows the trust model.

Recommended remediation priorities: None are urgent. The two Low findings can be addressed when convenient as hardening work; the Info finding is a documentation matter.

---

Findings

[LOW] lora-phy git dependency uses mutable branch ref, not a pinned commit

• File(s): Cargo.toml:35
• Category: Supply chain
• Status: Confirmed
• Severity: Low (CVSS-ish 4.4 — depends on a future cargo update and a maintainer-account compromise; reproducible builds are not affected today)
• CWE: CWE-829 (Inclusion of functionality from untrusted control sphere)

Summary

lora-phy is sourced from a git fork via branch = "fixed" rather than rev = "<sha>". Cargo.lock currently pins the specific commit 043d7659… (confirmed in Cargo.lock:632-640), so builds in this exact tree are reproducible. However, any future cargo update -p lora-phy (or running on a developer machine without a committed lockfile, which the firmware repo does commit) will silently advance to whatever sits on branch=fixed at that moment. If the swaits/lora-rs GitHub account were compromised, the next cargo update ingests the malicious commit and the next-built firmware ships it.

Data flow

1. Cargo.toml:35 declares lora-phy = { git = "https://github.com/swaits/lora-rs.git", branch = "fixed", features = ["defmt-03"] }.
2. Cargo.lock:634 records the current resolved commit hash.
3. Any contributor running cargo update (or any auto-update tooling on the maintainer's machine) replaces the lockfile entry with whatever is at HEAD of the branch at update time.
4. The resulting binary executes whatever the new HEAD commit contains, with no human-readable diff at the dependency boundary unless someone inspects the lockfile delta.

Proof of concept

Reproducibility today is fine — cargo build against the committed lockfile uses commit 043d7659…. The exposure is temporal: the moment cargo update -p lora-phy runs, the lockfile takes whatever commit is then on the branch. There is no PoC to demonstrate beyond reading the manifest.

Impact

A maintainer-account compromise (GitHub credential theft, PAT leak, repo-write malware) lets the attacker push a backdoored commit to the fixed branch. The next build that runs cargo update ships the backdoor into every flashed dongle. Severity is bounded by (a) the rate at which cargo update is run and (b) the difficulty of compromising the maintainer's GitHub. It's not zero — pinning to a commit closes the window.

Fix

Switch the manifest from branch = "..." to rev = "<commit_sha>":

lora-phy = { git = "https://github.com/swaits/lora-rs.git", rev = "043d7659dfe2148913f16b2f7ea3e9241642b770", features = ["defmt-03"] }

Re-pin (bump the rev) on each lora-phy upgrade. Drops the temporal exposure entirely; the dep changes only when a human edits the manifest. TODO.md already plans to drop the fork once upstream PRs merge — at that point switch back to a crates.io version, which gets the same effect from the registry's immutability.

---

[LOW] On lilygo_tbeam (ESP32 classic) only, esp-println output collides with host UART

• File(s): src/board/lilygo_tbeam.rs:112-120, Cargo.toml:73-77, src/main.rs:53-57
• Category: Wire-level reliability / information disclosure
• Status: Confirmed (frame corruption when error-level logs fire); not reachable from wire input today
• Severity: Low (reliability with disclosure side-effects; not directly attacker-triggerable)
• CWE: CWE-209 (Information Exposure Through an Error Message) — secondary

Summary

For the lilygo_tbeam build target (ESP32 classic, SX1276, CP2102-bridged UART), the host transport is UART0 on GPIO1/GPIO3. esp-println 0.17 is enabled with the auto (default) + defmt-espflash features. On ESP32 classic there is no USB-Serial-JTAG peripheral, so auto falls back to UART0 — the same UART the host protocol uses. Any defmt output (in production builds, only error! survives the compile-time filter) writes bytes onto the host wire mid-frame, corrupting the binary protocol the host is reading.

For heltec_v3_uart and elecrow_thinknode_m2 (ESP32-S3, which does have USB-Serial-JTAG), esp-println auto selects USB-Serial-JTAG — a different peripheral. Those boards are unaffected.

Data flow

1. Cargo.toml:73-77 pulls esp-println with defmt-espflash (no uart / jtag-serial feature explicitly set → auto picks at runtime).
2. src/main.rs:53-57 installs esp_println as defmt's global handler for the esp boards.
3. src/board/lilygo_tbeam.rs:113-120 initializes Uart::new(p.UART0, ...).with_tx(p.GPIO1).with_rx(p.GPIO3) as the host transport.
4. The ESP32 classic has no USB_SERIAL_JTAG peripheral → esp-println's runtime auto-selector falls back to UART0.
5. When a defmt::error!(...) or defmt::panic!(...) line executes on this board, esp-println writes bytes to UART0 → host sees them interleaved with COBS-framed protocol bytes.

Reachability from wire input

Walked the error! and panic! callsites: all fire only on hardware faults (Recovering: exhausted ... transitioning to Dead, radio chart entered Dead state, lora cold init failed, chart-invariant panics that are themselves not wire-reachable). None are reachable from a malformed command. Treat as a reliability concern, not a wire-attack.

Proof of concept

Run firmware compiled for lilygo_tbeam, intentionally induce a hardware-side error (e.g., disconnect the SX1276 SPI bus while the host is sending commands). Observe defmt error! output corrupting the COBS frames on the same UART. Without hardware tampering there is no PoC.

Impact

If the chip ever wedges hard enough to drive the chart into Recovering exhaustion or Dead, the host suddenly sees frame-CRC errors caused by defmt bytes instead of the chip-failure response it should have received. The host's framing layer recovers at the next clean 0x00 sentinel, but the lost frames could be the very ERadio reply that would have signaled the underlying failure. Diagnostic blindness during hardware failure on this one board variant.

Fix

Pick one of these for the lilygo_tbeam build:

• Option A (preferred): add "no-op" to the esp-println feature set for the lilygo_tbeam board feature, so the defmt global handler discards output entirely. Trade-off: no probe-rs defmt log capability on this board. Acceptable since the board has no SWD path anyway — it's CP2102-only.
• Option B: explicitly route esp-println to a different UART (e.g., UART1) by adding the uart feature with manual pin selection. Requires reading the esp-println 0.17 docs to confirm the pin override hook.

This is also the spirit of the existing debug-checkpoint feature for UART-only boards (the LED-blink workaround), per Cargo.toml:139-144. The same "no shared UART for protocol + logs" rule should apply.

---

[INFO] MAC address disclosed in USB serial descriptor and GET_INFO without authentication

• File(s): src/host/usb.rs:182-189, src/main.rs:159-161, 180-181
• Category: Information disclosure / privacy
• Status: Confirmed
• Severity: Info (intentional disclosure of a stable device identifier; standard for USB devices but worth documenting)
• CWE: CWE-200 (Exposure of Sensitive Information)

Summary

The device's 6-byte hardware MAC (nRF52840 FICR.DEVICEADDR / ESP32 eFuse / RP2040 flash-UID-derived) appears in two places, both readable without any authentication:

1. The USB CDC-ACM serial-number string descriptor (12 chars uppercase hex, no separator).
2. GET_INFO.mcu_uid (first 6 bytes; remaining 26 are zero).

Both surfaces are present at every boot. USB descriptors are read by every OS during enumeration without user consent; GET_INFO is wire-reachable by anyone who can write to the CDC-ACM/UART device.

Data flow (USB descriptor)

1. src/main.rs:99-109 — boot reads parts.mac via board::Board::mac_address().
2. src/main.rs:118-124 — MAC passed to host_task.
3. src/host/usb.rs:185-189 — MAC bytes formatted as 12 ASCII hex chars; assigned to config.serial_number.
4. Embassy USB stack publishes it as USB string descriptor (the OS retrieves on enumeration).

Data flow (GET_INFO)

1. src/main.rs:159-161 — mcu_uid[..6].copy_from_slice(&mac); Info.mcu_uid_len = 6.
2. Any host issuing a valid PING-tag-2-format GET_INFO frame gets the Info blob back, including the 6-byte MAC.

Impact

The MAC is a permanent, factory-burned device identifier. A user moving a dongle between hosts/networks (laptop ↔ desk; home ↔ office; conference Wi-Fi) is correlatable across contexts by any malicious userspace program that enumerates USB. The information is useful — donglora-mux and friends rely on it to distinguish multiple dongles — so it cannot simply be removed.

This is also the standard model for USB devices in general. Every USB device with a serial-number descriptor exposes a stable identifier; the user gets no consent prompt at enumeration time. Treat this finding as documentation rather than a remediation request.

Fix (optional, only if you care about cross-host de-anonymization)

• Replace the MAC in the USB serial descriptor with a per-install random byte string generated once and stored in MCU non-volatile memory (if available). Keep the real MAC accessible via authenticated GET_INFO if/when authentication is added.
• Or: leave the descriptor MAC alone (it's the standard pattern) and document the privacy expectation in README.md so users understand the dongle is trackable.

The Info-severity rating is the right call: this is a transparency / documentation question, not an actionable bug.

---

Appendix A — Out-of-scope items

The audit did not cover:

• Host-side applications that consume the protocol (ai-bot, bridge, client-py, mux-rs, mux-py, client-rs, tools). Each is a separate audit target.
• The donglora-protocol crate's invariants beyond how the firmware uses them. The sibling repo at /home/swaits/Code/donglora-org/protocol/ already maintains a fuzz suite (protocol/fuzz/) and mutation tests (protocol/mutants.out*); those validate the codec independent of this audit.
• Physical-layer / RF-regulatory compliance (FCC / CE / ETSI). The firmware is intentionally transparent; user is responsible.
• Side-channel analysis (timing, power, electromagnetic) — irrelevant on a device with no secrets.
• Build-time compromise (malicious build.rs, compromised toolchain, supply-chain attacks against rustc / espup / cargo). The Low finding above is the closest the audit gets to this surface.
• Bootloader / DFU surface (UF2 / Adafruit nRF52 bootloader / espflash). Those are upstream code paths; this audit covers the application image only.
• Hardware tampering / cold-boot RAM scraping / glitching the MCU. Out of scope for source-code audit.

Appendix B — Negative findings (audited and concluded not vulnerable)

Each item below was checked at source level and confirmed safe. This list is the most useful part of the audit: it documents what was examined and why a Critical / High finding does not exist.

• Frame decoder buffer handling (donglora-protocol-1.2.0/src/frame.rs::FrameDecoder::feed): fixed-size buf, bounded index, overflow → resync. No OOB. No panic. No unbounded growth.
• CRC-16/CCITT-FALSE on every frame: cryptographically weak (it's a CRC, not a MAC) but appropriate for transport integrity. Defends against the firmware acting on bit-flipped frames.
• Command::parse for every type_id (donglora-protocol-1.2.0/src/commands.rs:133-181): reserved bits, unknown enum values, length mismatches, empty payloads all rejected explicitly with typed errors that the firmware turns into ERR(ELENGTH|EPARAM|EMODULATION|EUNKNOWN_CMD). No silent acceptance.
• Modulation::decode and LoRaConfig::decode (donglora-protocol-1.2.0/src/modulation.rs:743-755, 224-263): every byte either bound-checked by from_u8 or match arms reject non-{0,1} booleans. WrongLength returns on size mismatch.
• validate_lora (radio.rs:2125-2145): freq, SF, BW, power, preamble bounds. The two unchecked fields (preamble_len upper bound and sync_word) were traced into downstream toa_us (u64 math with u32::MAX clamp — confirmed overflow-safe) and set_lora_sync_word (per memory-note, silent acceptance is intentional).
• try_start_rx_hw → prepare_for_rx chain (radio.rs:1348-1438): MAX_OTA_PAYLOAD as u8 = 255 is hardware-bounded (SX126x payload-length register is one byte). No truncation today; would surface at compile-time if MAX_OTA_PAYLOAD ever changed.
• collect_rx_packet (radio.rs:1664-1702): rx_buf: [u8; MAX_OTA_PAYLOAD] matches the chip-level cap. copy_len = len.min(MAX_OTA_PAYLOAD). extend_from_slice is bounds-safe.
• Panic reachability from wire input: 6 panic sites enumerated; all reach only via chart-shape violations, not from network input. Specifically, the NonZeroU16::new(tag).expect(...) at radio.rs:937 is gated by the if tag == 0 { ... return; } at host/framing.rs:36-40 before the envelope can be enqueued.
• unsafe blocks (4 sites): one-time-init static mut patterns (sound under stated preconditions, before any task spawns) and MMIO read_volatile / write_volatile (sound for RESETREAS / FICR.DEVICEADDR).
• Cross-task races / TOCTOU: embassy_sync::channel::Channel<CriticalSectionRawMutex, ...> for every cross-task hop. portable_atomic::AtomicBool with AcqRel swaps for shared flags. The radio chip has exactly one user (radio task), so no SPI-bus race.
• USB-descriptor / control-transfer handling: embassy_usb third-party. The firmware's CTRL_BUF = 128 is explicitly oversized to defeat a known panic site in the 0.5/0.6 era (documented in source comment at host/usb.rs:191-196).
• embassy_usb_driver::EndpointError handling (host/usb.rs:281-313): Disabled is logged at debug only (normal during enumeration), BufferOverflow at warn — and the decoder is reset and the read loop sleeps 100 ms to avoid burning the executor on a stuck driver.
• Stack overflow from nested parse calls: depth-bounded (feed → emit_decoded → Command::parse → Modulation::decode → LoRaConfig::decode); cumulative stack use ≈ 1.5 KB on a 4–32 KB task stack.
• DoS via async-err flood: RADIO_THROTTLED gates emit_async_err; outbound depth-64 ring absorbs bursts; the channel can never grow.
• DoS via Reset queue flood: RESET_PENDING AtomicBool dedupes injected resets; documented at channel.rs:16-29.
• Reachable defmt::panic!() in radio_task epilogue: gated by chart non-termination invariant (every leaf has a during: activity); not reachable by wire input.
• TX path payload handling: pending_tx.data is heapless::Vec<u8, MAX_OTA_PAYLOAD> — bounded at parse-time. lora.tx() writes whatever bytes the host supplied. No memory-corruption surface; no auth surface to bypass.
• OTA RX → outbound queue: the firmware does not interpret OTA bytes. Wire encoding (events.rs:228-249) is a fixed-layout memcpy with bound checks at the front (if buf.len() < total { Err }).
• InboundEvent::Reset clears the radio chartincoming_tag slot (radio.rs:923-930) so a tag-echoing action that runs after Reset doesn't pull a stale tag.
• No deserializers that take untrusted input by reflection (no serde, no XML, no YAML, no pickle, no protobuf with dynamic types).
• No SQL, NoSQL, shell, template, regex, HTTP, DNS, file, zip, XML surfaces in this firmware.

Appendix C — Confidence and methodology

• Confidence: High for the negative-findings list (every claim was traced with file:line); High for the three findings (each verified by re-reading the cited code after writing the draft).
• Method: MYTHOS Phase 1–5 walkthrough as defined in MYTHOS.md. Static review only; no runtime fuzzing was performed in this pass. The donglora-protocol crate's own fuzz suite (sibling repo) is presumed to validate codec invariants and is not duplicated here.
• Not done: cargo audit / cargo deny were not run as part of this audit. Recommended as a CI addition (CHANGELOG.md mentions hardening pulses; this would fit naturally). At the time of writing, no known reachable CVE for any used dependency surface is known to the auditor.

Self-check

• Every finding has file + line numbers
• Every Confirmed finding has a PoC, a data-flow trace, or an unambiguous "this is what's in the source today" reference (the Low / Info ratings make a remote PoC moot for the supply-chain and disclosure findings)
• Every finding has a one-sentence impact statement
• No findings are pure pattern-matches without reachability analysis
• Severity ratings are calibrated — nothing rated above Low; that matches what the audit actually found
• Negative findings appendix exists
• No suggestions beyond what's needed to fix the actual bug