AudioModule: validate codec2 header and bound the RX decode reads

[caveman99]

Two issues in the codec2 RX path, both reachable from a crafted AUDIO_APP payload (module is
ESP32 + 2.4 GHz / wide-LoRa only, so this affects those builds).

Unvalidated mode byte -> NULL dereference. When a received frame's header did not match
ours, the code built a temporary decoder from the frame's own mode byte:

CODEC2 *tmp_codec2 = codec2_create(audioModule->rx_encode_frame[3]);
codec2_set_lpc_post_filter(tmp_codec2, ...);   // tmp_codec2 == NULL for an invalid mode

codec2_create returns NULL for an out-of-range mode, and the following calls dereference it.
Now only frames that carry our own header (magic + mode) are decoded; anything else is dropped,
so an untrusted mode byte never reaches codec2_create.

Out-of-bounds read on a misaligned length. The decode loop stepped by the frame size but only
checked i < rx_encode_frame_index, so a payload whose length was not a clean multiple of the
frame size made the final codec2_decode read past the received data and potentially past the
233-byte rx_encode_frame. Each read is now bounded by i + frameSize <= rx_encode_frame_index
clamped to the buffer size, and the receive-side memcpy is clamped to the buffer as well.

Behavior change: a frame whose codec2 mode differs from this node's configured mode is dropped
rather than decoded with a temporary codec. Interoperable audio already requires both ends to run
the same codec2 bitrate, so cross-mode frames were not usefully decodable.

No test: the module is ESP32/SX1280-only and has no native build or existing test coverage, so
these changes are verified by inspection.

Summary by CodeRabbit

• Bug Fixes
  • Improved validation of incoming audio frames in RX mode before decoding, rejecting malformed or incompatible data.
  • Hardened TX-side audio capture to use byte-accurate buffering, improving reliability of captured frames.
  • Added strict bounds checking when copying received payloads to prevent overflows from crafted packets.
  • Removed fallback decoding on header mismatch to avoid unintended playback from invalid frames.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: efe0e024-2d4b-4760-988e-fb985a77a692

📥 Commits

Reviewing files that changed from the base of the PR and between acc4210 and 6d760be.

📒 Files selected for processing (1)

• src/modules/esp32/AudioModule.cpp

🚧 Files skipped from review as they are similar to previous changes (1)

• src/modules/esp32/AudioModule.cpp

---

📝 Walkthrough

Walkthrough

Audio RX processing bounds payload copies and validates frame headers and sizes before decoding. TX audio capture now accumulates I2S data using byte-based frame lengths and copies complete frames into the speech buffer.

Changes

AudioModule hardening

Layer / File(s)	Summary
RX frame validation and bounded copying src/modules/esp32/AudioModule.cpp	handleReceived limits payload copies to the RX buffer capacity, while run_codec2 validates frame bounds, headers, and codec sizing before decoding.
Byte-accurate TX audio capture src/modules/esp32/AudioModule.cpp	runOnce reads only remaining frame bytes, tracks byte-based accumulation, and copies complete captured frames into speech.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: thebentern

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately summarizes the main codec2 RX validation and read-bounding changes.
Description check	✅ Passed	The description covers the bugs, behavior change, affected hardware, and lack of tests, so it is mostly complete despite not using the template format.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch audio-frame-bounds

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/modules/esp32/AudioModule.cpp`:
- Around line 82-83: Update the i2s_write call in the audio output path to pass
the decoded buffer size in bytes by multiplying adc_buffer_size by
sizeof(int16_t), while leaving the buffer pointer and other arguments unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ef7b65b8-7210-477b-b9e6-81ff33e2bd57

📥 Commits

Reviewing files that changed from the base of the PR and between f18a8b0 and 8463da5.

📒 Files selected for processing (1)

• src/modules/esp32/AudioModule.cpp

[caveman99]

Addressed in acc4210. Confirmed and correct: adc_buffer_size is a sample count (codec2_samples_per_frame), and the TX path a few lines down already uses 2 * adc_buffer_size for its byte-sized memcpy, so i2s_write was being handed half the byte count. Now passing adc_buffer_size * sizeof(int16_t).

Two caveats: this is a pre-existing bug (the original i2s_write had the same sample count) that only shows up in this diff because the loop restructure moved the line, and I can't validate it on hardware - the module is ESP32/SX1280-only with no native build - so this is verified from the in-file sample/byte convention rather than by hearing corrected audio. Flagging in case you want to confirm on a real 2.4 GHz node.

[caveman99]

Follow-up: the sample/byte fix wasn't complete with just the write, so 6d760be fixes the mirror on the capture path.

There were three i2s spots in play:

• playback i2s_write (matched-header path) - CR's finding, fixed in acc4210.
• temp-codec i2s_write - removed entirely with the NULL-deref branch in the security fix, so nothing to fix there.
• capture i2s_read - the mirror bug, and worse: adc_buffer is uint16_t[] and adc_buffer_size a sample count, but i2s_read's size arg is bytes, adc_buffer_index accumulated bytes yet indexed a uint16_t* (advancing 2x), and the completion check compared bytes against samples. Net effect: it captured ~half a frame and copied the rest stale. Now byte-consistent throughout: frameBytes = adc_buffer_size * sizeof(uint16_t), a uint8_t* cursor, remaining-bytes read size, and completion/memcpy in bytes.

Same caveat as before: ESP32/SX1280-only, no native build, so both directions are verified from the in-module sample/byte convention rather than on hardware. This one reworks the capture state machine, so it's worth a real 2.4 GHz round-trip check before relying on it.

[github-actions]

⚡ Try this PR in the Web Flasher

Note

Building this pull request… the flash button, badges and supported-board
list will appear here automatically once CI finishes.