Skip to content

fix: bound what crafted audio files can make decoding and tagging do - #305

Merged
botandbang merged 4 commits into
mainfrom
bnb/untrusted-media
Oct 31, 2026
Merged

botandbang merged 4 commits into
mainfrom
bnb/untrusted-media

Conversation

@botandbang

Copy link
Copy Markdown
Contributor

What and why

This hardens three paths that trusted what a file's headers say.

Sample rates: decoding refuses a file whose header, or whose decoder, says more than 768 kHz (loclib_mixid::audio::MAX_RATE), next to the existing 0 Hz check. The fingerprint resampler keeps at most 16 MiB of cached filter weights. Above that it works each output's weights out again, with output identical to the bit. A crafted coprime rate used to grow the cache by about 17 floats per decoded sample.

M4A sample tables: symphonia-format-isomp4 0.5.5 reserves a table's memory from its claimed count before reading it, so a file claiming 4 billion samples asks for 16 GiB and aborts the process on Windows and on Linux without overcommit. The new loclib_mixid::audio::vet_container walks the atoms symphonia reads and refuses a table whose entries can't fit in its atom or the file, and a nested atom that runs past its parent or the file's end. audio::open (mix, check --audio, analysis, transcode) and the player's open_file call it. Test fixtures are generated in code. The player test's own M4A builder moved into test_util so both use one.

Serato tags: lofty writes its tag chunk where the RIFF/FORM size says the file ends. A size of 0 (a streamed recording) or one ending inside a chunk left a WAV or AIFF nothing could read. serato::marked_tag now refuses those at plan time, so the copy goes untagged with a warning. Trailing bytes after a correctly sized file are still tagged. A new property tries every size.

Removed: two TODO entries (the symphonia limit and the lofty RIFF size) and the property-test comment that steered around size 0.

Fixes LOC-43
Fixes #143
Fixes LOC-60
Fixes #161
Fixes LOC-38
Fixes #138

LOC-83 (#185) needs no change in this repo; it gets an upstream jacquard-oauth patch instead.

How it was tested

  • cargo fmt --check, and cargo clippy -p loclib-mixid -p loclib-interop -p locallibrary -p locallibrary-cli --all-targets -- -D warnings.
  • cargo nextest run -p loclib-mixid -E 'test(mp4::) | test(rates_past) | test(weight_cache)': 7 pass.
  • cargo nextest run -p loclib-interop -E 'test(header_sizes) | test(markers_refuse) | test(markers_written) | test(planned_as_written)': pass.
  • cargo nextest run -p locallibrary-cli -E 'test(fails_to_open)': 4 pass.
  • Not run here (disk was low): x86_64-pc-windows-msvc clippy and the full nextest run of the four crates.

Checklist

  • Tests cover the change
  • For locallibrary: apps/locallibrary/CHANGELOG.md has a line under Unreleased, if people using it will notice
  • The READMEs, docs/ and --help still say what the code does

Planned as these commits, folded into one because they share files:

- fix(loclib-mixid): refuse sample rates over 768 kHz and cap the resampler's weight cache
- fix(loclib-mixid): refuse an MP4 whose atoms claim more than the file holds before symphonia reads it
- fix(loclib-interop): refuse Serato tags for a WAV or AIFF whose header size ends inside its chunks
@botandbang
botandbang requested a review from bnb as a code owner October 30, 2026 16:15
@linear-code

linear-code Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

LOC-60

LOC-38

LOC-43

bnb
bnb previously approved these changes Oct 30, 2026
@github-actions github-actions Bot added documentation Improvements or additions to documentation locallibrary locallibrary and the loclib-* crates locallibrary: player The terminal player and its waveforms locallibrary: core The core crate, the data model and rekordbox's formats locallibrary: mixes Finding the tracks a recorded mix plays, and mix reports locallibrary: interop Libraries for Traktor, Serato and Engine DJ tests Tests, fixtures and test harnesses labels Oct 30, 2026
# Conflicts:
#	apps/locallibrary/CHANGELOG.md
#	crates/loclib-interop/src/serato.rs
bnb
bnb previously approved these changes Oct 30, 2026
@botandbang

Copy link
Copy Markdown
Contributor Author

Closing and reopening so CI reruns with the destinations test fix (#344) now on main. Branch unchanged.

@botandbang botandbang closed this Oct 31, 2026
auto-merge was automatically disabled October 31, 2026 00:29

Pull request was closed

@botandbang botandbang reopened this Oct 31, 2026
@botandbang
botandbang enabled auto-merge October 31, 2026 00:30
@botandbang botandbang closed this Oct 31, 2026
auto-merge was automatically disabled October 31, 2026 01:01

Pull request was closed

@botandbang botandbang reopened this Oct 31, 2026
@botandbang
botandbang enabled auto-merge October 31, 2026 01:01
@github-actions

Copy link
Copy Markdown

Rust coverage

From run 640. Both are held to the thresholds in .cargo/config.toml.

Tests Lines Functions Regions
All 97.43% (82995/85186) 96.48% (13222/13704) 95.68% (182525/190768)
Hegel property tests alone 95.57% (81409/85186) 94.24% (12914/13704) 93.66% (178665/190768)

@botandbang
botandbang merged commit 5c9b869 into main Oct 31, 2026
187 of 198 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation locallibrary: core The core crate, the data model and rekordbox's formats locallibrary: interop Libraries for Traktor, Serato and Engine DJ locallibrary: mixes Finding the tracks a recorded mix plays, and mix reports locallibrary: player The terminal player and its waveforms locallibrary locallibrary and the loclib-* crates tests Tests, fixtures and test harnesses

Projects

None yet

2 participants