mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
test(buzz-media): add truncation-sweep instrument and its vacuity trap
Found a remotely reachable panic in the new WAV validator: five raw bytes[offset + N..] indexes guarded only by the declared fmt chunk length, not by bytes actually present. A 22-byte upload reaches it and 14 of 16 truncation points in the fmt body panic, before auth runs. The first version of this sweep reported the WAV fixture clean against a validator already proven to panic. Truncating a real WAV leaves the RIFF size field stale, so declared + 8 == len rejects every input a few lines into the walker. A three-stage probe over 400 prefixes measured it: naive enters the function 396 times and reaches the vulnerable reads 0 times; repairing the size field yields 378 reaches. Keep both arms so the contrast stays visible, and instrument the line under test rather than the function containing it. Co-authored-by: Dawn <c6237ef84fa537c78dcee78efd2d4e59f728859c7f194da42ac51ededfa0be05@buzz.block.builderlab.xyz> Signed-off-by: Dawn <c6237ef84fa537c78dcee78efd2d4e59f728859c7f194da42ac51ededfa0be05@buzz.block.builderlab.xyz>
This commit is contained in:
@@ -84,3 +84,46 @@ Every decoder ignores that field on header pages, so the file decoded, timed,
|
||||
and PCM-compared perfectly while being wire-invalid. An oracle is only as
|
||||
strong as its strictest clause; when the relay validator rejects something
|
||||
this script blesses, the script is what's wrong.
|
||||
|
||||
## Instrument 5: truncation sweep (`dawn_trunc_sweep.rs`)
|
||||
|
||||
Added 2026-08-14 after finding a remotely reachable panic in the relay's WAV
|
||||
validator (`validation.rs:252-258`): five raw `bytes[offset + N..]` indexes
|
||||
guarded only by the *declared* `fmt ` chunk length, never by bytes actually
|
||||
present. A 22-byte upload reached it; 14 of 16 truncation points in the fmt
|
||||
body panicked. Reachable before auth (`upload.rs:82-85` validates ahead of
|
||||
`verify_blossom_upload_auth`), and the repo has no `CatchPanicLayer`.
|
||||
|
||||
### The vacuity trap — READ BEFORE WRITING A TRUNCATION SWEEP
|
||||
|
||||
Truncating a real WAV leaves the RIFF size field stale, so
|
||||
`declared + 8 == bytes.len()` rejects every input a few lines into the walker
|
||||
and the sweep never reaches the code under test. Measured with a three-stage
|
||||
probe over 400 prefixes:
|
||||
|
||||
```text
|
||||
naive enter=396 past_size_gate=0 at_fmt_fields=0
|
||||
riff-repaired enter=396 past_size_gate=388 at_fmt_fields=378
|
||||
```
|
||||
|
||||
The naive sweep enters the function 396 times and reaches the vulnerable
|
||||
reads ZERO times. It reported a confident `OK` against a validator already
|
||||
proven to panic. **Any container with a self-describing length field must have
|
||||
that field repaired to match the truncated length**, or the sweep is vacuous
|
||||
while looking thorough. For WAV: rewrite `bytes[4..8] = (len - 8) as u32` after
|
||||
truncating. Keep BOTH arms in the test so the contrast stays visible.
|
||||
|
||||
"Entered the function" is not "reached the code." Instrument the specific
|
||||
line you care about, not the function containing it.
|
||||
|
||||
### Why MP3/Ogg were clean (construction, not luck)
|
||||
|
||||
- MP3 (`:164-219`): every header via `.get(offset..offset + 4).ok_or(...)?`;
|
||||
`frame_len` bounded with `checked_add` + `end > bytes.len()` before advancing.
|
||||
- Ogg (`:322-355`): binds `header` via `.get(offset..offset + 27).ok_or(...)?`
|
||||
FIRST, so `header[6..14]` etc. are in-bounds off a checked slice.
|
||||
|
||||
That is the pattern WAV is missing: it checks a prefix, then indexes the
|
||||
ORIGINAL buffer past it. Fix shape is one whole-body slice —
|
||||
`bytes.get(offset + 8..offset + 24).ok_or(MetadataForbidden)?` — then slice
|
||||
the six fields from it. Removes the class, not the instance.
|
||||
|
||||
@@ -0,0 +1,52 @@
|
||||
fn cfg() -> buzz_media::config::MediaConfig {
|
||||
buzz_media::config::MediaConfig {
|
||||
s3_endpoint: String::new(), s3_access_key: String::new(),
|
||||
s3_secret_key: String::new(), s3_bucket: String::new(),
|
||||
s3_region: "us-east-1".into(),
|
||||
s3_addressing_style: buzz_media::config::S3AddressingStyle::Path,
|
||||
max_image_bytes: 50*1024*1024, max_gif_bytes: 10*1024*1024,
|
||||
max_video_bytes: 524_288_000, max_file_bytes: 104_857_600,
|
||||
max_audio_bytes: 26_214_400,
|
||||
public_base_url: "http://localhost:3000/media".into(),
|
||||
upload_records_enabled: false, upload_ip_header: None, upload_port_header: None,
|
||||
}
|
||||
}
|
||||
|
||||
fn sweep(name: &str, full: &[u8], repair_riff: bool) -> Option<usize> {
|
||||
let mut first = None;
|
||||
let lens: Vec<usize> = (0..full.len().min(600)).chain((600..full.len()).step_by(97)).collect();
|
||||
for n in lens {
|
||||
let mut slice = full[..n].to_vec();
|
||||
// Keep the RIFF declared size consistent with the truncated length, or
|
||||
// the `declared + 8 == len` gate rejects everything before the fmt walk
|
||||
// and the sweep never reaches the code under test.
|
||||
if repair_riff && slice.len() >= 8 {
|
||||
let d = (slice.len() - 8) as u32;
|
||||
slice[4..8].copy_from_slice(&d.to_le_bytes());
|
||||
}
|
||||
let r = std::panic::catch_unwind(|| {
|
||||
let _ = buzz_media::validation::validate_file_content(&slice, &cfg());
|
||||
});
|
||||
if r.is_err() && first.is_none() { first = Some(n); }
|
||||
}
|
||||
match first {
|
||||
None => println!("{name:24} OK (no panic at any prefix)"),
|
||||
Some(n) => println!("{name:24} *** first panic at prefix {n} ***"),
|
||||
}
|
||||
first
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dawn_truncation_sweep_all_containers() {
|
||||
let dir = std::path::PathBuf::from(std::env::var("DAWN_SAN").unwrap());
|
||||
let mut bad = Vec::new();
|
||||
for name in ["tagged.mp3","mpeg2_22k.mp3","mpeg25_11k.mp3","tagged.ogg","bigart.ogg","long.ogg"] {
|
||||
let full = std::fs::read(dir.join(name)).unwrap();
|
||||
if sweep(name, &full, false).is_some() { bad.push(name); }
|
||||
}
|
||||
// WAV twice: naive (declared size left stale) and repaired (reaches fmt walk).
|
||||
let wav = std::fs::read(dir.join("tagged.wav")).unwrap();
|
||||
if sweep("tagged.wav (naive)", &wav, false).is_some() { bad.push("wav-naive"); }
|
||||
if sweep("tagged.wav (riff-repaired)", &wav, true).is_some() { bad.push("wav-repaired"); }
|
||||
assert!(bad.is_empty(), "validator panicked: {bad:?}");
|
||||
}
|
||||
Reference in New Issue
Block a user