diff --git a/CHANGELOG.md b/CHANGELOG.md index ce5fc79c..7accf389 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,11 @@ # Changelog -## [1.6.15] — unreleased +## [1.7.0] — 2026-09-02 ### Fixed +- `mux/mp4`: the capped child-box scan treated any `size < 8` as a stop, which dropped a 64-bit `largesize` child box (`size == 1`, real length in the following 8 bytes) and every sibling after it; a `size == 0` (run-to-end) box was mishandled too. Both are now honoured, so an MP4/MOV whose boxes use 64-bit sizes is walked fully instead of truncated. + - Linux: `SgIoTransport::raw_command` clamped an over-length CDB with `cdb.len().min(16)` instead of routing it through the shared `checked_cdb_len` guard that `execute()` uses. Under SPC-4 a CDB's length is fixed by its opcode, so truncating one does not shorten the command — it sends a descriptor the drive will read as something else. An empty CDB was likewise unguarded, and went to the sg driver as a zero-length command descriptor. Unreachable — `raw_command` is private and its one caller passes a fixed 6-byte ALLOW MEDIUM REMOVAL — but it was the one transport path that did the thing the guard exists to prevent. - Linux: the fd hand-off between a background recovery thread and `SgIoTransport::drop` had no release edge, so a recovery thread that published its fd after teardown had drained the slot was not guaranteed to observe `dead == true` and could leave the descriptor unclosed. `Drop`'s claiming `swap` was `Acquire` (a relaxed store, heading no release sequence) and the recovery thread's `compare_exchange` was `Release` (a relaxed load, acquiring nothing) — neither side alone was enough, and upgrading only one still leaks. Both are now `AcqRel`. Worst case was a leaked file descriptor on a transport already being torn down. diff --git a/Cargo.toml b/Cargo.toml index 78feb441..db01577a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "libfreemkv" -version = "1.6.14" +version = "1.7.0" edition = "2024" rust-version = "1.88" license = "MIT" diff --git a/src/mux/mp4/read.rs b/src/mux/mp4/read.rs index 6440b0af..3299fb3a 100644 --- a/src/mux/mp4/read.rs +++ b/src/mux/mp4/read.rs @@ -426,7 +426,7 @@ fn find_boxes_capped<'a>(payload: &'a [u8], want: &[u8; 4], cap: usize) -> Vec<& let mut out = Vec::new(); let mut pos = 0; while pos + 8 <= payload.len() && out.len() < cap { - let size = u32::from_be_bytes([ + let size32 = u32::from_be_bytes([ payload[pos], payload[pos + 1], payload[pos + 2], @@ -438,13 +438,36 @@ fn find_boxes_capped<'a>(payload: &'a [u8], want: &[u8; 4], cap: usize) -> Vec<& payload[pos + 6], payload[pos + 7], ]; - if size < 8 || pos + size > payload.len() { + // ISO/IEC 14496-12 §4.2: size==1 → 64-bit largesize after the type + // (16-byte header); size==0 → box runs to the payload end. The child + // scan must honour both or a largesize sibling ends the walk early. + let (box_size, header_len) = match size32 { + 1 => { + if pos + 16 > payload.len() { + break; + } + let large = u64::from_be_bytes([ + payload[pos + 8], + payload[pos + 9], + payload[pos + 10], + payload[pos + 11], + payload[pos + 12], + payload[pos + 13], + payload[pos + 14], + payload[pos + 15], + ]) as usize; + (large, 16usize) + } + 0 => (payload.len() - pos, 8usize), + n => (n, 8usize), + }; + if box_size < header_len || pos + box_size > payload.len() { break; } if &bt == want { - out.push(&payload[pos + 8..pos + size]); + out.push(&payload[pos + header_len..pos + box_size]); } - pos += size; + pos += box_size; } out } @@ -998,6 +1021,28 @@ mod tests { } } + // Regression: a child box in the 64-bit largesize form (size==1) must not + // stop the sibling walk — old code saw size 1 < 8 and broke, so every box + // after it (here `moov`) vanished and its track was silently dropped. + #[test] + fn find_box_walks_past_a_largesize_sibling() { + let mut payload = Vec::new(); + // `free` as an empty largesize box: size32=1, u64 total size = 16. + payload.extend_from_slice(&1u32.to_be_bytes()); + payload.extend_from_slice(b"free"); + payload.extend_from_slice(&16u64.to_be_bytes()); + // `moov` (32-bit) with a 4-byte payload immediately after it. + payload.extend_from_slice(&12u32.to_be_bytes()); + payload.extend_from_slice(b"moov"); + payload.extend_from_slice(&[0xDE, 0xAD, 0xBE, 0xEF]); + + assert_eq!( + find_box(&payload, b"moov"), + Some(&[0xDEu8, 0xAD, 0xBE, 0xEF][..]), + "moov after a largesize `free` sibling must still be found", + ); + } + // Positive-path test for `parse_stss`: two distinct entries in a buffer // sized to exactly fit them, catching offset/stride arithmetic mutants. // See docs/mp4-read.md — parse_stss_reads_two_distinct_entries.