Repository navigation
fix: bound what crafted audio files can make decoding and tagging do - #305
Merged
Merged
Conversation
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
bnb
previously approved these changes
Oct 30, 2026
# Conflicts: # apps/locallibrary/CHANGELOG.md # crates/loclib-interop/src/serato.rs
botandbang
enabled auto-merge
October 30, 2026 22:02
bnb
previously approved these changes
Oct 30, 2026
bnb
approved these changes
Oct 30, 2026
Contributor
Author
|
Closing and reopening so CI reruns with the destinations test fix (#344) now on main. Branch unchanged. |
auto-merge was automatically disabled
October 31, 2026 00:29
Pull request was closed
botandbang
enabled auto-merge
October 31, 2026 00:30
auto-merge was automatically disabled
October 31, 2026 01:01
Pull request was closed
botandbang
enabled auto-merge
October 31, 2026 01:01
Rust coverageFrom run 640. Both are held to the thresholds in .cargo/config.toml.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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_containerwalks 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'sopen_filecall it. Test fixtures are generated in code. The player test's own M4A builder moved intotest_utilso 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_tagnow 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, andcargo 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.x86_64-pc-windows-msvcclippy and the full nextest run of the four crates.Checklist
apps/locallibrary/CHANGELOG.mdhas a line under Unreleased, if people using it will noticedocs/and--helpstill say what the code does