Skip to content

Commit 2d2c13e

Browse files
author
crispasr integration
committed
fix(stb_vorbis): heap overflow on untrusted audio — the allocation truncates, the consumer does not
Closes the PLAN item OPEN since 2026-08-18. Reachable from crispasr_audio_load, so the CLI, the server's upload path and every binding. THE MECHANISM, confirmed by arithmetic against the original ASAN report: f->comment_list_length = get32_packet(f); // attacker u32 f->comment_list = setup_malloc(f, sizeof(char*) * length); // int param memset(f->comment_list, 0, sizeof(char*) * length); // size_t setup_malloc's parameter is an `int`. With length = 1646854400 the product is 13,174,835,200, which truncates to 289,933,312 — EXACTLY the allocation size the report showed. The memset computes the same product, but `sizeof` makes it a size_t, so it is the full 13,174,835,200 — EXACTLY the write size the report showed. One quantity, computed two ways, disagreeing across a boundary. UPSTREAM HAS NOT FIXED IT. nothings/stb master still has `setup_malloc(vorb *f, int sz)` and the same truncating multiply, so there was nothing to pull (which is what the OPEN item asked to check first) and upstream is vulnerable by the same truncation — lacking our memset, its `for` loop instead walks comment_list[i] past the short allocation. Worth reporting there. THE MEMSET IS ITSELF AN EARLIER CRISPASR PATCH, added to fix a different fuzz crash (vorbis_deinit freeing an unwritten array). It did not create the truncation; it converted a gradual overflow into an immediate 13 GB one. THE FIX, two layers: 1. At the call site, bound the count before the multiply. Two bounds: the product must not truncate (INT_MAX/sizeof(char*), minus setup_malloc's (sz+7)&~7 rounding), and — derived from the data rather than picked — every comment costs at least 4 bytes for its own u32 length, so a file of N bytes cannot declare more than N/4 comments. That cannot reject a real file: one with C comments necessarily carries >= 4*C bytes. stream_len is 0 for stdio-backed streams, where only the first bound applies. 2. At the choke point, setup_malloc rejects a negative sz. Nearly every caller passes `sizeof(T) * count` computed in size_t, so this is not a one-off shape; a truncated value landing negative would otherwise reach malloc as a huge size_t. The vendor string and the per-comment strings on the same path have the same shape (attacker u32 length, then `len+1` in an int), so length_is_plausible() bounds them by the same data-derived rule. REPRODUCER, and it is verified in both directions. tests/fuzz/regressions/ogg-comment-count-int-overflow.ogg, 101 bytes, generated by tools/gen-ogg-comment-fuzz-seed.py. Against the pre-fix vendored decoder under ASAN it reproduces the three-week-old report EXACTLY: WRITE of size 13174835200, 0 bytes after a 289933312-byte region, start_decoder:3683, setup_malloc:960, stb_vorbis_open_memory:5141, stb_vorbis_decode_memory:5419 — every size and every line number. After the fix the same input is rejected cleanly. Reachability through crispasr_audio_load is established by the original report's own stack, which is where it was found. AND A CORRECTION TO THE CORPUS README. The seed it documented as missing declared comment_list_length = 0x3FFFFFFF. I generated it and it reproduces NOTHING against current code: 8 * 0x3FFFFFFF truncates to -8, so setup_malloc fails and the header is rejected. Restoring that exact value would have added a seed that gates nothing — the same disease as the empty replay set it was written to fix. That row is retired rather than restored, with the reason.
1 parent cfbf5a6 commit 2d2c13e

4 files changed

Lines changed: 103 additions & 4 deletions

File tree

examples/stb_vorbis.c

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -947,8 +947,29 @@ static void *make_block_array(void *mem, int count, int size)
947947
return p;
948948
}
949949

950+
// CrispASR patch (security): is a declared byte-length plausible for this
951+
// stream? Every string in the comment header is preceded by its own u32 length
952+
// and must actually be present, so a declared length longer than the whole file
953+
// is a malformed header, not a big string. Derived from the data, so it cannot
954+
// reject a real file. stream_len is 0 for stdio-backed streams; there we can
955+
// only fall back to the int-overflow bound at the call site.
956+
static int length_is_plausible(vorb *f, int len)
957+
{
958+
if (len < 0) return FALSE;
959+
if (f->stream_len > 0 && (uint32) len > f->stream_len) return FALSE;
960+
return TRUE;
961+
}
962+
950963
static void *setup_malloc(vorb *f, int sz)
951964
{
965+
// CrispASR patch (security): the parameter is an int, and nearly every
966+
// caller passes `sizeof(T) * count` computed in size_t, so a large
967+
// attacker-controlled count truncates on the way in — and can land
968+
// negative. Callers then size their loops from the UNtruncated count.
969+
// The comment_list case is bounded at its own call site (see start_decoder);
970+
// this is the choke point that stops any other call site failing the same
971+
// way, and a negative sz would otherwise reach malloc as a huge size_t.
972+
if (sz < 0) return NULL;
952973
sz = (sz+7) & ~7; // round up to nearest 8 for alignment of future allocs.
953974
f->setup_memory_required += sz;
954975
if (f->alloc.alloc_buffer) {
@@ -3650,6 +3671,10 @@ static int start_decoder(vorb *f)
36503671
if (!vorbis_validate(header)) return error(f, VORBIS_invalid_setup);
36513672
//file vendor
36523673
len = get32_packet(f);
3674+
// CrispASR patch (security): same class as comment_list below — `len` is an
3675+
// attacker-controlled u32 and `len+1` can overflow int before it is even
3676+
// passed on. Reject an implausible length instead of allocating for it.
3677+
if (!length_is_plausible(f, len) || len == INT_MAX) return error(f, VORBIS_invalid_setup);
36533678
f->vendor = (char*)setup_malloc(f, sizeof(char) * (len+1));
36543679
if (f->vendor == NULL) return error(f, VORBIS_outofmem);
36553680
for(i=0; i < len; ++i) {
@@ -3661,7 +3686,38 @@ static int start_decoder(vorb *f)
36613686
f->comment_list = NULL;
36623687
if (f->comment_list_length > 0)
36633688
{
3664-
f->comment_list = (char**) setup_malloc(f, sizeof(char*) * (f->comment_list_length));
3689+
// CrispASR patch (security): BOUND THE COUNT BEFORE THE MULTIPLY.
3690+
//
3691+
// setup_malloc takes an `int`, so `sizeof(char*) * comment_list_length`
3692+
// is computed as size_t and then TRUNCATED on the way in, while the
3693+
// memset below computes the same product and does not. Found by
3694+
// linux-fuzz-smoke: with comment_list_length = 1646854400 the product is
3695+
// 13,174,835,200, truncating to 289,933,312 — and ASAN reported exactly
3696+
// that allocation size and exactly that write size. The allocation
3697+
// truncates; the consumer does not.
3698+
//
3699+
// Two bounds, cheapest first:
3700+
// 1. The count must be small enough that the product cannot truncate.
3701+
// setup_malloc also rounds up with (sz+7)&~7, so leave that room.
3702+
// 2. Every comment costs at least 4 bytes in the packet — its own u32
3703+
// length field — so a file of N bytes cannot legitimately declare
3704+
// more than N/4 comments. This is derived from the data rather than
3705+
// being a magic number, and it cannot reject a real file: one with
3706+
// C comments necessarily carries at least 4*C bytes. stream_len is 0
3707+
// for stdio-backed streams, so that arm is skipped there and bound 1
3708+
// still applies.
3709+
//
3710+
// Upstream nothings/stb has the same truncating call and no fix as of
3711+
// 2026-09-07; it lacks this memset, so instead of one huge write its
3712+
// loop below walks comment_list[i] past the short allocation.
3713+
int max_comments = (int) (((size_t) INT_MAX - 7u) / sizeof(char*));
3714+
if (f->stream_len > 0 && (f->stream_len / 4u) < (uint32) max_comments)
3715+
max_comments = (int) (f->stream_len / 4u);
3716+
if (f->comment_list_length > max_comments) {
3717+
f->comment_list_length = 0; // keep "length describes the array" true
3718+
return error(f, VORBIS_invalid_setup);
3719+
}
3720+
f->comment_list = (char**) setup_malloc(f, (int) (sizeof(char*) * (size_t) f->comment_list_length));
36653721
// CrispASR patch: the length is already the ATTACKER-CONTROLLED value read
36663722
// above, so returning here with a NULL array leaves the two out of step and
36673723
// vorbis_deinit walks `comment_list_length` entries of a null pointer. Keep
@@ -3685,6 +3741,8 @@ static int start_decoder(vorb *f)
36853741

36863742
for(i=0; i < f->comment_list_length; ++i) {
36873743
len = get32_packet(f);
3744+
// CrispASR patch (security): as for the vendor string above.
3745+
if (!length_is_plausible(f, len) || len == INT_MAX) return error(f, VORBIS_invalid_setup);
36883746
f->comment_list[i] = (char*)setup_malloc(f, sizeof(char) * (len+1));
36893747
if (f->comment_list[i] == NULL) return error(f, VORBIS_outofmem);
36903748

tests/fuzz/regressions/README.md

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,22 @@ where that was practical, so it is deterministic and small enough to read.
1515
saying so. The glob matched nothing on every run from then on, and an empty
1616
replay set is indistinguishable from a passing one. `.gitignore` in this
1717
directory now re-includes the audio extensions, and the CI step counts what it
18-
copied and fails at zero. The ogg seed itself is LOST and has to be re-crafted
19-
from the description below before that row is a gate again.
18+
copied and fails at zero.
19+
20+
**The lost ogg seed was re-crafted, and testing it produced a correction to the
21+
row below.** Its documented value, `comment_list_length = 0x3FFFFFFF`, does NOT
22+
reproduce anything against current code: `8 * 0x3FFFFFFF` truncates to −8, so
23+
`setup_malloc` fails and the header is rejected cleanly. Restoring that exact
24+
value would have put a seed in the corpus that gates nothing — the same shape as
25+
the empty replay set it was meant to fix. The value that *does* reproduce is
26+
`1646854400`, which is the one committed. `tools/gen-ogg-comment-fuzz-seed.py`
27+
generates either; pass a count to explore the space.
2028

2129
| File | Bug | Fixed in |
2230
|---|---|---|
2331
| `wav-1hz-resample-oom.wav` | 344 KB, harvested from the run that found it (CI run 33840954769). A structurally valid RIFF/WAVE declaring `sampleRate = 1`. miniaudio resamples it to the 16 kHz target — 16000x — so 176 000 stored samples become 2.816e9 output frames, 11.3 GB, and the chunked decode loops doubled their buffer with no ceiling. Reachable from any surface that accepts a user file, including server upload. | `src/crispasr_audio.cpp`, `crispasr_max_decoded_frames()` bounds decoded frames against input size at all three loops |
24-
| `ogg-huge-comment-count.ogg` **(MISSING — see above)** | 102 bytes. Ogg/Vorbis comment header declaring `comment_list_length = 0x3FFFFFFF`. The allocation of `sizeof(char*) * length` fails, and stb_vorbis returned from the error path with the length still set and `comment_list` NULL — `vorbis_deinit` then indexed the null array. ASAN: `SEGV in vorbis_deinit`, reached from `crispasr_audio_load`. | `examples/stb_vorbis.c`, guard in `vorbis_deinit` + reset the length on the error path |
32+
| `ogg-comment-count-int-overflow.ogg` | 101 bytes, crafted (`tools/gen-ogg-comment-fuzz-seed.py`). Ogg/Vorbis comment header declaring `comment_list_length = 1646854400`. `setup_malloc` takes an **int**, so `sizeof(char*) * length` = 13,174,835,200 truncates to 289,933,312 and allocates that; the `memset` on the next line computes the same product in `size_t` and writes the full 13 GB. The allocation truncates, the consumer does not. Verified: against the pre-fix vendored decoder it reproduces the original report exactly — same write size, same region size, same `start_decoder:3683` / `setup_malloc:960` / `open_memory:5141` / `decode_memory:5419` frames — and is rejected cleanly after. | `examples/stb_vorbis.c`, bound the count before the multiply + reject a negative `sz` in `setup_malloc` |
33+
| ~~`ogg-huge-comment-count.ogg`~~ **(retired — its value gates nothing, see above)** | 102 bytes. Ogg/Vorbis comment header declaring `comment_list_length = 0x3FFFFFFF`. The allocation of `sizeof(char*) * length` fails, and stb_vorbis returned from the error path with the length still set and `comment_list` NULL — `vorbis_deinit` then indexed the null array. ASAN: `SEGV in vorbis_deinit`, reached from `crispasr_audio_load`. | `examples/stb_vorbis.c`, guard in `vorbis_deinit` + reset the length on the error path |
2534

2635
## Adding one
2736

101 Bytes
Binary file not shown.

tools/gen-ogg-comment-fuzz-seed.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
#!/usr/bin/env python3
2+
"""Craft the minimal Ogg that reaches stb_vorbis's comment_list allocation.
3+
4+
No CRC is computed: stb_vorbis does not validate the page CRC (and its serial
5+
check is commented out), so the bytes that matter are the page framing, the
6+
identification packet it insists on, and the comment header's two u32 lengths.
7+
"""
8+
import struct, sys, pathlib
9+
10+
def page(seq, flags, payload):
11+
segs, rest = [], len(payload)
12+
while rest >= 255:
13+
segs.append(255); rest -= 255
14+
segs.append(rest)
15+
return (b"OggS" + bytes([0, flags]) + struct.pack("<q", 0)
16+
+ struct.pack("<I", 0x1234) + struct.pack("<I", seq)
17+
+ struct.pack("<I", 0) # CRC: not checked
18+
+ bytes([len(segs)]) + bytes(segs) + payload)
19+
20+
# Identification packet — every field must survive stb_vorbis's validation or we
21+
# never reach the comment header. blocksize nibbles must be 6..13 with log0<=log1.
22+
ident = (b"\x01vorbis" + struct.pack("<I", 0) + bytes([1]) + struct.pack("<I", 44100)
23+
+ struct.pack("<iii", 0, 0, 0) + bytes([(8 << 4) | 8]) + bytes([1]))
24+
assert len(ident) == 30, len(ident)
25+
26+
COUNT = int(sys.argv[2]) if len(sys.argv) > 2 else 1646854400
27+
comment = b"\x03vorbis" + struct.pack("<I", 0) + struct.pack("<I", COUNT)
28+
29+
out = pathlib.Path(sys.argv[1])
30+
out.write_bytes(page(0, 0x02, ident) + page(1, 0x00, comment))
31+
prod = 8 * COUNT
32+
print(f"count={COUNT} 8*count={prod} truncated to int32={prod % 2**32} file={out.stat().st_size}B")

0 commit comments

Comments
 (0)