tools/ax25_replay.cpp: heap overflow in loadWav() when the WAV data chunk length is not a multiple of 4
Nobody has claimed this yet.
Assessment
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Newbie friendliness
- 64/100
Research direction
Start in tools/ax25_replay.cpp:60-82 and review the existing WAV bounds and float32 validation, then inspect CMakeLists.txt:4768 and try building the ax25_replay target. Choose and document whether partial samples are rejected or truncated, ensure allocation and copying use consistent sizes, and handle the noted bounds concern. Done means the malformed WAV no longer causes an overflow and the relevant test or verification passes.
Written by the indexing model from the issue text.
Description
loadWav() in tools/ax25_replay.cpp sizes its destination buffer by rounding
the WAV data chunk length down to whole samples, then memcpys the
full declared length into it. A data chunk whose length is not a multiple
of 4 overflows the heap allocation by up to 3 bytes.
// tools/ax25_replay.cpp:81-82
samples.resize(static_cast<size_t>(dataBytes / 4)); // rounds DOWN
std::memcpy(samples.data(), data, static_cast<size_t>(dataBytes)); // copies FULL length
dataBytes == 6 allocates one float (4 bytes) and copies 6.
Reproduced under ASan
The data length is fully attacker-controlled, and this needs no malformed
framing to reach: the file is otherwise well-formed, every byte it declares is
really present, and the existing bounds check at :60 passes cleanly.
==68511==ERROR: AddressSanitizer: heap-buffer-overflow
WRITE of size 6 at 0x7b87f7de0074 thread T0
#0 memcpy
#1 loadWav(...) ax25_replay.cpp:82
0x7b87f7de0074 is located 0 bytes after 4-byte region [...0070,...0074)
allocated by thread T0 here:
#6 std::vector<float>::resize(unsigned long)
#7 loadWav(...) ax25_replay.cpp:81
The generator for the input file — a valid mono float32 WAV whose data chunk
declares 6 bytes:
import struct
fmt = struct.pack('<HHIIHH', 3, 1, 48000, 48000*4, 4, 32) + b'\x00\x00'
data = b'\x01\x02\x03\x04\x05\x06' # 6 bytes = 1.5 floats
body = b'WAVE' + b'fmt ' + struct.pack('<I', len(fmt)) + fmt \
+ b'data' + struct.pack('<I', len(data)) + data
wav = b'RIFF' + struct.pack('<I', len(body)) + body
open('evil.wav','wb').write(wav + b'\x00' * max(0, 44 - len(wav)))
Caveat on how this was run: ax25_replay does not currently link (see
"Second, unrelated" below), so I ran loadWav() and its two readLe* helpers
extracted verbatim from the file, compiled with -fsanitize=address against
Qt6Core. The extracted code is byte-identical to tools/ax25_replay.cpp:26-84;
only main() was replaced. Worth re-confirming against the real binary once it
builds again.
Note on the Antares advisory that surfaced this
This was surfaced by the Antares detector on PR #4905 (which does not touch this
file). Its stated mechanism is not correct — it reports that the code "does
not verify that pos + sz <= bytes.size()", but :60-61 does exactly that and
breaks, so an oversized data length cannot reach the memcpy. The real
defect is the resize/memcpy size mismatch above, which is adjacent to but
distinct from what was reported. Flagging so the advisory isn't closed as a
false positive along with its mechanism.
Fix is a judgment call — hence an issue, not a PR
Two defensible behaviours, and the choice belongs to whoever owns the AX.25
tooling:
- Reject a
datalength that isn't a multiple of 4 — it is a malformed
float32 WAV, and the tool already refuses non-float32/non-mono input with a
clear error, so this is consistent with how it treats thefmtchunk. - Truncate to whole samples —
memcpydataBytes & ~3and ignore the
trailing partial sample.
Either way the two sizes must be derived from one expression rather than
two. There is also a latent 32-bit-only concern: pos + static_cast<qsizetype>(sz)
with sz near UINT32_MAX can wrap where qsizetype is 32-bit, which would
defeat the :60 check — not reachable on the 64-bit builds anyone uses, but
worth handling in the same pass.
Scope and severity
Deliberately not fixed in #4905 — unrelated to that PR, and folding a
memory-safety fix into a feature branch makes both harder to review.
Mitigating: ax25_replay is an EXCLUDE_FROM_ALL developer tool
(CMakeLists.txt:4768), not built by default and not shipped, so exposure is a
developer pointing it at a malformed WAV — not a path reachable from the running
app or from the radio. Constitution Principle VII (untrusted input validated at
the boundary) still applies: a contributor-supplied file is exactly the input
class it names.
Pre-existing since #3381 (June); not a regression.
Second, unrelated: the tool does not currently build
cmake --build build --target ax25_replay fails to link on main
(f6f56865), on both a Debug and a clean RelWithDebInfo tree:
undefined reference to `AetherSDR::HdlcCodec::processBit(unsigned char)'
undefined reference to `AetherSDR::HdlcCodec::bitstreamSize() const'
from src/core/tnc/AetherAx25LibmodemShim.cpp:836 / :1022. Being
EXCLUDE_FROM_ALL, CI never builds it, so the breakage is invisible to the
gate. Happy to split this into its own issue if you'd rather keep them separate.
- Dominant language
- C++
- Stars
- 221
- Forks
- 117
- Avg merge
- 2d 7h
- Merged PRs (30d)
- 299
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from aethersdr/AetherSDR
-
audio documentation good first issue maintainer-review
Difficulty 1/5 Under an hour Newbie friendliness 92/100
-
bug CW maintainer-review safety
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
-
docs(agents): family PRs must stay in the family tree (shared applet/pan/model edits stall review) Opendocumentation enhancement maintainer-review
Difficulty 1/5 1-3 hours Newbie friendliness 90/100
-
bug good first issue GUI maintainer-review priority: low
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
-
bug
Difficulty 1/5 Under an hour Newbie friendliness 86/100
All issues in aethersdr/AetherSDR
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 86/100
-
Sensor initialization takes very long when `--initial-sim-time` is set to current UNIX timestamp Open
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
gazebosim/gz-sensors#662 · 1 comment ·
-
enhancement
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
-
comp-datalake
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
ClickHouse/ClickHouse#121222 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
LadybirdBrowser/ladybird#12123 ·