Audit hardening: untrusted-file/IPC safety + WASAPI sample-rate fix#18
Merged
Conversation
A crafted audio file could drive multi-gigabyte allocations during import/probe, before a single sample is read: - read_wav_header allocated `vec![0u8; len]` from the `fmt ` chunk's untrusted 32-bit length (up to 4 GiB). Reject lengths outside the 16..=MAX_WAV_FMT_CHUNK_BYTES range before allocating. - The peak-LOD builders and load_rauf's `vec![0u8; byte_len]` trusted header frame counts (WAV `data` chunk size, RAUF `frames_written`). Clamp them to the bytes actually present on disk in generate_wav_peaks_streaming, generate_rauf_peaks_streaming, and load_rauf. The byte-slice path (wav_data_layout) already validated chunk bounds; this brings the File-based paths to parity. Adds regression tests for the absurd-fmt-length and corrupt-frames_written cases. Co-Authored-By: Claude Opus 4.8 <[email protected]>
Exclusive mode passed the device mix format (native rate) to IAudioClient::Initialize unchanged, but stored the *requested* rate as the engine sample rate. A requested rate that differed from the device native rate left the hardware running at one rate while the engine believed another — silent pitch/tempo desync across transport, meters, and recording. Negotiate the real rate instead: stamp the requested rate into the mix format (nSamplesPerSec + nAvgBytesPerSec), probe IsFormatSupported in exclusive mode, and fall back to the native rate (with a log) when the device rejects it. The reported sample_rate now always equals the rate the hardware runs at, and both the initial and aligned-retry periods derive from it. Co-Authored-By: Claude Opus 4.8 <[email protected]>
run_isolated_format_scan only routed AudioUnit through the scanner child; VST3/CLAP fell through to run_inprocess_scan, loading untrusted plugin binaries directly in the host. catch_unwind can't stop a C++ access violation, so one malformed plugin crashed the app despite the "isolated" name. (The app's bulk scan uses run_isolated_bundle_scan, which was already isolated, but this public API was a footgun.) Prefer the out-of-process scanner for every format when the scanner binary is present; keep in-process as a best-effort fallback for builds without it. Add a format-generic ScannerProcessCrashed error so a crashed VST3/CLAP child no longer reports as "AudioUnit". Co-Authored-By: Claude Opus 4.8 <[email protected]>
CreateFileMappingW opens the existing section (returning a valid handle) when the named region already exists, signalling it only via ERROR_ALREADY_EXISTS. The creator side never checked it, so a same-session process that pre-created the predictably-named section would have the engine map a region it didn't own. Ring indices are masked/clamped so this was a DoS/correctness risk, not memory-unsafety — fail closed on the creator side instead. Adds a Windows regression test. Co-Authored-By: Claude Opus 4.8 <[email protected]>
read_frame used read_line with no ceiling, so a stuck or hostile peer could grow the line buffer without limit via one newline-less giant line (base64 plugin-state blobs travel this path). Cap each frame at MAX_FRAME_BYTES via a take()-bounded read; split out read_frame_limited so the bound is unit-testable without allocating the production limit. Co-Authored-By: Claude Opus 4.8 <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
1 participant
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.
Lead-engineer security/robustness sweep of all non-gpui crates (2026-06-19). Six findings, all fixed. Common theme: trust the real file/OS state, not attacker-controllable header/IPC values.
Fixes (one commit each, except #2/#4 share a file)
backend/wasapi_exclusive.rs) — exclusive mode passed the device mix format (native rate) toInitializeunchanged but stored the requested rate as the engine rate → hardware ran one rate while the engine believed another (silent pitch/tempo desync). Now stamps the requested rate intomix_fmt(nSamplesPerSec+nAvgBytesPerSec), probesIsFormatSupported, and falls back to native if unsupported; period/aligned-period derive from the chosen rate.fmtchunk OOM (audio_file.rs) —read_wav_headerallocatedvec![0u8; len]from an untrusted 32-bit chunk length (up to 4 GiB) before reading. Reject lengths outside16..=MAX_WAV_FMT_CHUNK_BYTES.scan/isolation.rs,scan/types.rs) —run_isolated_format_scanonly isolated AudioUnit; VST3/CLAP loaded plugin binaries in-process (catch_unwindcan't stop a C++ access violation). Now prefers the scanner subprocess for every format, in-process fallback only when the binary is missing. Added a format-genericScannerProcessCrashederror.audio_file.rs) — LOD builders andload_rauf'svec![0u8; byte_len]trusted header frame counts (WAVdatasize, RAUFframes_written). Clamp to the bytes actually on disk.audio_bridge.rs) —CreateFileMappingWopens an existing section (valid handle) on name collision, signalling only viaERROR_ALREADY_EXISTS, which the creator never checked. Now fails closed. (Ring indices were already masked/clamped, so this was DoS/correctness, not memory-unsafety.)ipc/mod.rs) —read_frameusedread_linewith no ceiling. Cap each frame atMAX_FRAME_BYTES(128 MiB) via atake()-bounded read.Tests
audio_file::peak_tests— absurdfmtlength rejected; corruptframes_writtenclamped.ipc::tests— oversized frame rejected; within-limit frame round-trips.audio_bridge::tests::create_named_rejects_squatted_name(Windows).cargo check+cargo test --libpass for bothsphere-direct-audio-engineandsphere-plugin-host.Not runtime-verified
🤖 Generated with Claude Code