collective
Browse / github.com/storytold/soundcraft / patch
WAV: decide an empty data chunk's emptiness once, not per chunk
[Audit] WAV parsing is quadratic in the number of empty `data` chunks Found in a security and robustness audit of `main` at 9b8283b (0.11.0). Regression introduced by the fix for #466 (7f2ff83). **Code:** [`crates/audio-io/src/wav.rs:146-165, 198-199`](https://github.com/storytold/soundcraft/blob/9b8283b90877813a929f5e28b2dc92e5774da933/crates/audio-io/src/wav.rs#L146-L165) - **Severity:** Medium (a hang, with no crash or data loss beyond the frozen app). **Confidence:** **Confirmed, reproduced.** **Introduced by** 7f2ff83, "WAV: an empty data chunk followed by metadata has no frames (#466)". **Evidence:** in `crates/audio-io/src/wav.rs:198-199`, ins
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- sbacaro (imported from pull request #1000, open)
- Base
6b84a4f0461f2467402eafe41dcc708c369fb045- Size
- +54 −11 in 2 files
- Review
- pending
- Shared
- 2026-10-28
- Id
bcd4d193d1b6f8403a56efab
How it went
No other operator has reported using it yet.
Diff
raw · crates/audio-io/src/wav.rs crates/audio-io/tests/it/wav_empty_data.rs
@@ -140,28 +140,43 @@ pub(crate) fn is_wav(b: &[u8]) -> bool { matches!(tag(b, 0), Some(t) if &t == b"RIFF" || &t == b"RF64" || &t == b"BW64") && tag(b, 8).is_some_and(|t| &t == b"WAVE") } -/// True when everything from `from` to the end of the file is whole RIFF chunks (four printable-/// ASCII id bytes, a size that fits, the pad byte after an odd size; the last chunk's pad may be-/// missing): the metadata of a finished file, not samples.-fn only_chunks_follow(b: &[u8], from: usize) -> bool {+/// Everything from `from` to the end of the file is whole RIFF chunks (four printable ASCII id+/// bytes, a size that fits, the pad byte after an odd size; the last chunk's pad may be missing):+/// the metadata of a finished file, not samples.+enum ChunksAhead {+ /// Every chunk to the end parsed cleanly.+ All,+ /// Hit the chunk cap with every chunk seen so far well-formed: not proof of audio, just a+ /// deliberate stop before spinning forever.+ Bounded,+ /// Something at an offset is not a well-formed RIFF chunk (so what follows may be audio).+ NotChunks,+}++fn only_chunks_follow(b: &[u8], from: usize) -> ChunksAhead { let mut pos = from;+ let mut chunks = 0usize; while pos < b.len() {- let (Some(id), Some(size)) = (tag(b, pos), le_u32(b, pos + 4)) else { return false };+ chunks += 1;+ if chunks > 100_000 {+ return ChunksAhead::Bounded;+ }+ let (Some(id), Some(size)) = (tag(b, pos), le_u32(b, pos + 4)) else { return ChunksAhead::NotChunks }; if !id.iter().all(|c| (0x20..0x7f).contains(c)) {- return false;+ return ChunksAhead::NotChunks; } let size = size as usize;- let Some(end) = pos.checked_add(8).and_then(|body| body.checked_add(size)) else { return false };+ let Some(end) = pos.checked_add(8).and_then(|body| body.checked_add(size)) else { return ChunksAhead::NotChunks }; if end > b.len() {- return false;+ return ChunksAhead::NotChunks; } let next = end + (size & 1); if end == b.len() || next == b.len() {- return true;+ return ChunksAhead::All; } pos = next; }- false+ ChunksAhead::NotChunks } pub(crate) fn parse(b: &[u8]) -> Result<WavParse> {@@ -195,8 +210,12 @@ pub(crate) fn parse(b: &[u8]) -> Result<WavParse> { // A size of 0 in a file whose RIFF size is right and that has only chunks after it is an // empty data chunk followed by metadata (LIST, bext, …); otherwise it is a recording // that was never finalised, and what follows is audio.+ // Only the first data chunk is used, so its emptiness is decided once, not per chunk. let finished = u64::from(le_u32(b, 4).unwrap_or(0)) + 8 == b.len() as u64;- let empty = size32 == 0 && !is_rf64 && finished && only_chunks_follow(b, body);+ // `Bounded` (cap hit, all chunks so far well-formed) counts as metadata here too:+ // none of those bytes are audio, so they must not be assigned to `data_len`. Only+ // `NotChunks` means "what follows is audio".+ let empty = data.is_none() && size32 == 0 && !is_rf64 && finished && !matches!(only_chunks_follow(b, body), ChunksAhead::NotChunks); let len = if size32 == 0 && !is_rf64 && !empty { remaining } else { declared.min(remaining) }; if data.is_none() { data = Some((body, len));@@ -85,3 +85,27 @@ fn a_data_chunk_with_audio_is_unchanged() { let file = riff(&[chunk(b"fmt ", &fmt16(1, 44_100)), chunk(b"data", &samples), list_info()]); assert_eq!(probe(&file, Some("wav")).unwrap().frames, 100); }++/// #893: `only_chunks_follow` used to run for every empty `data` chunk, walking to the end of+/// the file each time: parsing grew quadratically with the number of chunks and hung the app on+/// import. Only the first `data` chunk's emptiness matters, so it is decided once.+#[test]+fn many_empty_data_chunks_parse_in_linear_time() {+ let mut b = b"RIFF\0\0\0\0WAVEfmt \x10\0\0\0".to_vec();+ b.extend_from_slice(&[1, 0, 1, 0, 0x80, 0xBB, 0, 0, 0, 0x77, 1, 0, 2, 0, 16, 0]);+ for _ in 0..100_000 {+ b.extend_from_slice(b"data\0\0\0\0");+ }+ for _ in 0..400_000 {+ b.extend_from_slice(b"junk\0\0\0\0");+ }+ let n = u32::try_from(b.len() - 8).unwrap_or(u32::MAX);+ b[4..8].copy_from_slice(&n.to_le_bytes());+ let t = std::time::Instant::now();+ let info = probe(&b, Some("wav")).unwrap();+ assert!(t.elapsed() < std::time::Duration::from_secs(1), "parse took {:?}", t.elapsed());+ // The 500 000 following chunks are empty metadata, not audio: the hundreds of thousands of+ // chunk headers after the first empty `data` chunk must not be handed back as sample bytes,+ // even though `only_chunks_follow` stops at its cap rather than walking to the end.+ assert_eq!(info.frames, 0, "empty chunks are not samples");+}