From b47bccce7b1514cf2856728d51273be313148766 Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Sat, 19 Sep 2026 16:19:48 -0700 Subject: [PATCH] libfreemkv 1.7.4: codeaudit fixes (v1.7.0..HEAD sweep) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bus content-range de-bus: - F1 disc/mod.rs: always install content ranges (extracted wire_bus_removal); empty ranges => Some([]) => de-bus NOTHING (fail-safe), not None => de-bus everything — closes silent clear-sector corruption on the AACS cert route. - F4 aacs/content.rs, F5 decrypt.rs: content gate is now a partition_point binary search (O(log n)/sector) over the sorted, merged-disjoint ranges, replacing the per-sector O(ranges) linear scan on the de-bus hot path. - F2 scsi/linux.rs: RECOVERY_THREADS cap uses atomic reserve-then-check (fetch_add first, fall back on overshoot) — removes the TOCTOU that let concurrent multi-drive rips exceed MAX_RECOVERY_THREADS. - F3 pes.rs: PesFrame::deserialize reads each chunk straight into data's tail; drops the scratch Vec + double-copy, keeps the incremental-growth DoS guard. - F7 disc/mod.rs: six inline rank_titles thresholds -> named consts. Tests: bus fail-safe + scan wiring (both routes), per-sector gate straddle, read_directory type-3 continuation/AED-skip/cycle-guard, strengthened without_bus_removal no-op guard. F6 (pub decrypt_bus) left by-design. --- src/aacs/content.rs | 17 +++++- src/decrypt.rs | 23 +++++++- src/disc/mod.rs | 72 ++++++++++++++++++----- src/drive/mod.rs | 65 +++++++++++++++++++++ src/pes.rs | 21 ++++--- src/scsi/linux.rs | 44 +++++++------- src/sector/bus_removal.rs | 25 ++++++++ src/udf.rs | 118 ++++++++++++++++++++++++++++++++++++++ 8 files changed, 333 insertions(+), 52 deletions(-) diff --git a/src/aacs/content.rs b/src/aacs/content.rs index 6e6f5dd5..476476c3 100644 --- a/src/aacs/content.rs +++ b/src/aacs/content.rs @@ -293,12 +293,23 @@ pub(crate) fn decrypt_bus_in_content( base_lba: u32, ranges: Option<&[(u32, u32)]>, ) { + debug_assert!( + ranges.is_none_or(|rs| rs.windows(2).all(|w| w[0].0 <= w[1].0)), + "content ranges must be sorted ascending by start LBA for the binary search" + ); let in_content = |lba: u32| -> bool { match ranges { None => true, - Some(rs) => rs - .iter() - .any(|&(start, cnt)| lba >= start && (lba as u64) < start as u64 + cnt as u64), + // `rs` is sorted ascending by start and non-overlapping (merged + // extents), so binary-search the last range with start <= lba and + // test containment — O(log n) per sector, not O(ranges). + Some(rs) => { + let i = rs.partition_point(|&(start, _)| start <= lba); + i > 0 && { + let (start, cnt) = rs[i - 1]; + (lba as u64) < start as u64 + cnt as u64 + } + } } }; let cipher = crate::aacs::crypto::new_cipher_for(read_data_key); diff --git a/src/decrypt.rs b/src/decrypt.rs index 8a5a6c33..b04cb44f 100644 --- a/src/decrypt.rs +++ b/src/decrypt.rs @@ -367,9 +367,17 @@ pub(crate) fn decrypt_sectors_mapped_in_content( /// mapped decrypt so clear filesystem/nav units outside every encrypted-content /// extent are passed through untouched. fn lba_in_content_ranges(lba: u32, ranges: &[(u32, u32)]) -> bool { - ranges - .iter() - .any(|&(start, count)| lba >= start && (lba - start) < count) + debug_assert!( + ranges.windows(2).all(|w| w[0].0 <= w[1].0), + "content ranges must be sorted ascending by start LBA for the binary search" + ); + // Sorted, non-overlapping (merged extents): binary-search the last range + // whose start <= lba and test containment — O(log n), not O(ranges). + let i = ranges.partition_point(|&(start, _)| start <= lba); + i > 0 && { + let (start, count) = ranges[i - 1]; + (lba - start) < count + } } // AACS scheme step: apply `map`'s per-unit keys to `buf`, in-place — no key @@ -1193,6 +1201,10 @@ mod tests { BusRemovalSectorSource::new(WireSource { bytes: wire }, BusStage::Passthrough); let mut buf = vec![0u8; aacs::content::ALIGNED_UNIT_LEN]; src.read_sectors(0, 3, &mut buf, false).unwrap(); + // Snapshot the still-bus-encrypted wire as delivered, so we can prove the + // unit-key decrypt actually TOUCHED the buffer (not a silent no-op) — a + // bare `!= clear` also passes on an untouched buffer. + let wire_delivered = buf.clone(); let keys = DecryptKeys::Aacs { unit_keys: vec![(0, unit_key)], @@ -1206,6 +1218,11 @@ mod tests { buf, clear, "without bus removal first, unit-key decrypt alone cannot recover clear" ); + assert_ne!( + buf, wire_delivered, + "unit-key decrypt must have actually run over the buffer (not a no-op); \ + it just can't recover clear without bus removal first" + ); } // ── FMTS phase-aware map ────────────────────────────────────────────────── diff --git a/src/disc/mod.rs b/src/disc/mod.rs index d8a27469..d3a5e07b 100644 --- a/src/disc/mod.rs +++ b/src/disc/mod.rs @@ -492,6 +492,29 @@ pub(crate) fn aacs_dir_present(udf_fs: &crate::udf::UdfFs) -> bool { udf_fs.find_dir("/AACS").is_some() || udf_fs.find_dir("/BDMV/AACS").is_some() } +// Title-ranking heuristic thresholds. Each gates a DISTINCT decision — several +// share a value (0.5) by coincidence, so they are named and tuned separately. +// See docs/disc-mod.md for the shape gate. +/// Nav candidate: a real-video title qualifies only if it runs at least this +/// fraction of the longest probable-video title's duration. +const NAV_CANDIDATE_MIN_DURATION_FRAC: f64 = 0.5; +/// Payload floor: a promoted title must hold at least this fraction of the +/// largest video-bearing title's byte size, else a tiny playback-valid branch +/// could win nav/authoring. +const FEATURE_PAYLOAD_MIN_SIZE_FRAC: f64 = 0.10; +/// Composite/wrapper gate: candidate wrapper Q must be at least this fraction of +/// the outer title P's byte size to count as a plausible wrapper. +const WRAPPER_MIN_SIZE_FRAC: f64 = 0.5; +/// Seamless-branch guard (issue #45): Q is a wrapper if it is a much SHORTER cut, +/// i.e. its duration is below this fraction of P's duration. +const SEAMLESS_SHORTER_CUT_DURATION_FRAC: f64 = 0.85; +/// Seamless-branch guard (issue #45): Q is a wrapper if it is nearly P's SIZE, i.e. +/// its byte size is at least this fraction of P's (a bumper, not a real branch). +const SEAMLESS_BUMPER_SIZE_FRAC: f64 = 0.90; +/// Authoring hint: honoured only when the title runs at least this fraction of the +/// longest probable-video title's duration. +const AUTHORING_MIN_DURATION_FRAC: f64 = 0.5; + /// Union a set of extents into sorted, merged, disjoint `(start_lba, /// sector_count)` ranges — the pure, testable core of /// [`Disc::encrypted_content_ranges`]. Reuses [`crate::udf::merge_ranges`]. @@ -1749,15 +1772,9 @@ impl Disc { tracing::info!(target: "freemkv::scan", titles = disc.titles.len(), format = ?disc.content_format, "phase: titles parsed"); // Wire the SINGLE de-bus point onto the drive from the handshake result, - // BEFORE the caller samples keys or muxes — gated to encrypted-content - // extents (the earlier UDF/metadata reads ran under default Passthrough). - session.set_bus_stage(crate::sector::bus_removal::BusStage::from_read_data_key( - bus_key, - )); - let content_ranges = disc.encrypted_content_ranges(); - if !content_ranges.is_empty() { - session.set_bus_content_ranges(std::sync::Arc::from(content_ranges.into_boxed_slice())); - } + // BEFORE the caller samples keys or muxes (the earlier UDF/metadata reads + // ran under default Passthrough). + Self::wire_bus_removal(session, bus_key, disc.encrypted_content_ranges()); // No CSS key recovery at scan time: DVD CSS keys are per-title and are // re-cracked keylessly at read/decrypt time (`disc.css` stays unset), @@ -1766,6 +1783,25 @@ impl Disc { Ok(disc) } + /// Wire the SINGLE AACS bus-removal de-bus point onto `session` from a + /// completed handshake. The cert-route Read Data Key (`bus_key`, `None` on + /// the firmware/vendor route) picks the `BusStage`; the encrypted-content + /// ranges gate it. ALWAYS install the ranges even when empty: `None` means + /// "de-bus every sector", `Some([])` means "de-bus none" — so a host-key disc + /// that parsed zero content extents fails SAFE (untouched) instead of de-bussing + /// clear UDF/nav bytes to garbage. Extracted from [`Self::scan`] so it is + /// unit-testable without a live SCSI handshake. + pub(crate) fn wire_bus_removal( + session: &mut Drive, + bus_key: Option<[u8; 16]>, + content_ranges: Vec<(u32, u32)>, + ) { + session.set_bus_stage(crate::sector::bus_removal::BusStage::from_read_data_key( + bus_key, + )); + session.set_bus_content_ranges(std::sync::Arc::from(content_ranges.into_boxed_slice())); + } + // The extents an image-time CSS crack scans, in the crate's CANONICAL // order (main feature's own extents, playback order) — do not re-derive // this inline. See docs/disc-mod.md — image_crack_extents. @@ -2057,7 +2093,9 @@ impl Disc { .iter() .filter(|t| { t.has_video() - && (longest_video_dur <= 0.0 || t.duration_secs >= 0.5 * longest_video_dur) + && (longest_video_dur <= 0.0 + || t.duration_secs + >= NAV_CANDIDATE_MIN_DURATION_FRAC * longest_video_dur) }) .map(|t| t.playlist_id) .collect(); @@ -2238,7 +2276,8 @@ impl Disc { .max() .unwrap_or(0); let carries_feature_payload = |t: &DiscTitle| { - max_video_size == 0 || t.size_bytes as f64 >= 0.10 * max_video_size as f64 + max_video_size == 0 + || t.size_bytes as f64 >= FEATURE_PAYLOAD_MIN_SIZE_FRAC * max_video_size as f64 }; // The composite scan below is O(n^2 * clip-set-size); title/clip counts are // untrusted, so a crafted image could stall it. Bound the total pair work; @@ -2266,12 +2305,14 @@ impl Disc { !qj.is_empty() && qj.len() < pi.len() && q.has_probable_video() - && q.size_bytes as f64 >= 0.5 * p.size_bytes as f64 + && q.size_bytes as f64 >= WRAPPER_MIN_SIZE_FRAC * p.size_bytes as f64 // Seamless-branch guard (issue #45): wrapper only // if Q is a much SHORTER cut (concat) or nearly P's // SIZE-equal (bumper). See docs/disc-mod.md. - && (q.duration_secs < 0.85 * p.duration_secs - || q.size_bytes as f64 >= 0.90 * p.size_bytes as f64) + && (q.duration_secs + < SEAMLESS_SHORTER_CUT_DURATION_FRAC * p.duration_secs + || q.size_bytes as f64 + >= SEAMLESS_BUMPER_SIZE_FRAC * p.size_bytes as f64) && qj.is_subset(pi) } }); @@ -2281,7 +2322,8 @@ impl Disc { let authoring = !composite && hint.is_some_and(|h| h.matches(p.playlist_id, &p.playlist)) && p.has_video() - && (longest_video_dur <= 0.0 || p.duration_secs >= 0.5 * longest_video_dur) + && (longest_video_dur <= 0.0 + || p.duration_secs >= AUTHORING_MIN_DURATION_FRAC * longest_video_dur) && carries_feature_payload(p); // Nav result is video- and payload-gated like the authoring hint, // so a streamless or low-byte branch/DV presentation cannot win diff --git a/src/drive/mod.rs b/src/drive/mod.rs index 82003ef8..f981f53b 100644 --- a/src/drive/mod.rs +++ b/src/drive/mod.rs @@ -1711,6 +1711,71 @@ mod command_tests { ); } + // F1 fail-safe: an AacsHostKey stage with an EMPTY content map must de-bus + // NOTHING (empty `Some([])`), not everything — Disc::scan always installs the + // map now, so a host-key disc that parsed no titles leaves clear sectors alone. + #[test] + fn bus_empty_content_ranges_debus_nothing() { + use crate::sector::SectorSource; + let rdk = [0x7Eu8; 16]; + let clear = clear_bus_content(2); + let mut wire = clear.clone(); + crate::aacs::content::encrypt_bus(&mut wire, &rdk); + let mut d = drive_with(wire.clone()); + d.set_bus_stage(crate::sector::bus_removal::BusStage::AacsHostKey(rdk)); + // Empty map = "no content anywhere" → de-bus nothing (fail-safe). + d.set_bus_content_ranges(std::sync::Arc::from( + Vec::<(u32, u32)>::new().into_boxed_slice(), + )); + let mut got = vec![0u8; clear.len()]; + d.read_sectors(300, 2, &mut got, false).unwrap(); + assert_eq!( + got, wire, + "empty content map must leave every sector untouched (still bus-encrypted), \ + never de-bus clear bytes into garbage" + ); + } + + // scan()'s handshake→bus_stage wiring, via the `wire_bus_removal` seam. CERT + // route: a Some(rdk) handshake arms AacsHostKey + installs the content map, so + // an in-range bus-encrypted sector is de-bussed to plaintext. + #[test] + fn wire_bus_removal_cert_route_arms_host_key_and_ranges() { + use crate::sector::SectorSource; + let rdk = [0x6Bu8; 16]; + let clear = clear_bus_content(1); + let mut wire = clear.clone(); + crate::aacs::content::encrypt_bus(&mut wire, &rdk); + let mut d = drive_with(wire); + crate::disc::Disc::wire_bus_removal(&mut d, Some(rdk), vec![(300u32, 3u32)]); + let mut got = vec![0u8; clear.len()]; + d.read_sectors(300, 1, &mut got, false).unwrap(); + assert_eq!( + got, clear, + "Some(read_data_key) must arm AacsHostKey + ranges so content is de-bussed" + ); + } + + // FIRMWARE/vendor route: a None handshake maps to Passthrough, so even an + // in-range sector is NOT de-bussed. MUTATION: mapping None to AacsHostKey + // would corrupt these bytes here. + #[test] + fn wire_bus_removal_firmware_route_is_passthrough() { + use crate::sector::SectorSource; + let rdk = [0x6Bu8; 16]; + let clear = clear_bus_content(1); + let mut wire = clear.clone(); + crate::aacs::content::encrypt_bus(&mut wire, &rdk); // still bus-encrypted on the wire + let mut d = drive_with(wire.clone()); + crate::disc::Disc::wire_bus_removal(&mut d, None, vec![(300u32, 3u32)]); + let mut got = vec![0u8; clear.len()]; + d.read_sectors(300, 1, &mut got, false).unwrap(); + assert_eq!( + got, wire, + "None must map to Passthrough — no host-side de-bus, bytes returned verbatim" + ); + } + #[test] fn read_capacity_normal_adds_one() { // last_lba = 0x0000_0063 (99) → capacity 100 sectors. diff --git a/src/pes.rs b/src/pes.rs index 65ca7bbb..ef1a4583 100644 --- a/src/pes.rs +++ b/src/pes.rs @@ -141,18 +141,17 @@ impl PesFrame { if len > MAX_FRAME_SIZE { return Err(crate::error::Error::PesFrameTooLarge { size: len }.into()); } - // Grow the buffer INCREMENTALLY (not `vec![0u8; len]`): `len` is attacker-controlled - // up to MAX_FRAME_SIZE (256 MiB), so a truncated stream claiming a huge frame must not - // pre-allocate the max; a bounded scratch chunk tracks delivered bytes, short reads still surface UnexpectedEof, valid frames assemble byte-for-byte. - const GROW_CHUNK: usize = 1024 * 1024; // 1 MiB scratch, heap-allocated + // Grow INCREMENTALLY (not `vec![0u8; len]`): `len` is attacker-controlled to 256 MiB, so + // a truncated stream claiming a huge frame must not pre-allocate the max. Read each chunk + // straight into `data`'s own tail — no scratch buffer, no second copy; short reads EOF. + const GROW_CHUNK: usize = 1024 * 1024; // 1 MiB growth step let mut data: Vec = Vec::with_capacity(len.min(GROW_CHUNK)); - let mut chunk = vec![0u8; GROW_CHUNK.min(len).max(1)]; - let mut remaining = len; - while remaining > 0 { - let want = remaining.min(chunk.len()); - r.read_exact(&mut chunk[..want])?; - data.extend_from_slice(&chunk[..want]); - remaining -= want; + let mut filled = 0usize; + while filled < len { + let want = (len - filled).min(GROW_CHUNK); + data.resize(filled + want, 0); + r.read_exact(&mut data[filled..filled + want])?; + filled += want; } Ok(Some(Self { track, diff --git a/src/scsi/linux.rs b/src/scsi/linux.rs index 10e40dd1..5331d33d 100644 --- a/src/scsi/linux.rs +++ b/src/scsi/linux.rs @@ -338,24 +338,25 @@ impl ScsiTransport for SgIoTransport { let recovery = self.fd_recovery.clone(); let dead = self.dead.clone(); - // Cap outstanding recovery threads: past MAX_RECOVERY_THREADS a - // sustained wedge would spawn unbounded threads, so fall back to - // doing the close/reopen inline on this thread instead. + // Cap outstanding recovery threads (past MAX a sustained wedge spawns + // unbounded threads): reserve the slot ATOMICALLY via fetch_add, act on + // the pre-increment value, and give the slot back if it overshoots. use std::sync::atomic::Ordering; - if RECOVERY_THREADS.load(Ordering::Acquire) < MAX_RECOVERY_THREADS { - RECOVERY_THREADS.fetch_add(1, Ordering::AcqRel); + if RECOVERY_THREADS.fetch_add(1, Ordering::AcqRel) < MAX_RECOVERY_THREADS { std::thread::spawn(move || { if old_fd >= 0 { unsafe { libc::close(old_fd) }; } RECOVERY_THREADS.fetch_sub(1, Ordering::AcqRel); }); - } else if old_fd >= 0 { - unsafe { libc::close(old_fd) }; + } else { + RECOVERY_THREADS.fetch_sub(1, Ordering::AcqRel); + if old_fd >= 0 { + unsafe { libc::close(old_fd) }; + } } - if RECOVERY_THREADS.load(Ordering::Acquire) < MAX_RECOVERY_THREADS { - RECOVERY_THREADS.fetch_add(1, Ordering::AcqRel); + if RECOVERY_THREADS.fetch_add(1, Ordering::AcqRel) < MAX_RECOVERY_THREADS { std::thread::spawn(move || { // Don't unwrap: a device path with an interior NUL would // panic this detached thread (silently swallowed). Bail @@ -383,17 +384,20 @@ impl ScsiTransport for SgIoTransport { } RECOVERY_THREADS.fetch_sub(1, Ordering::AcqRel); }); - } else if let Ok(c_path) = std::ffi::CString::new(path.as_os_str().as_encoded_bytes()) { - // At the recovery-thread cap: reopen inline (blocking this - // call briefly) instead of spawning an unbounded 9th thread. - let new_fd = unsafe { - libc::open( - c_path.as_ptr() as *const libc::c_char, - libc::O_RDWR | libc::O_NONBLOCK | libc::O_CLOEXEC, - ) - }; - if new_fd >= 0 { - self.fd = new_fd; + } else { + // Over the cap: give the reservation back, then reopen inline + // (blocking this call briefly) instead of spawning a 9th thread. + RECOVERY_THREADS.fetch_sub(1, Ordering::AcqRel); + if let Ok(c_path) = std::ffi::CString::new(path.as_os_str().as_encoded_bytes()) { + let new_fd = unsafe { + libc::open( + c_path.as_ptr() as *const libc::c_char, + libc::O_RDWR | libc::O_NONBLOCK | libc::O_CLOEXEC, + ) + }; + if new_fd >= 0 { + self.fd = new_fd; + } } } diff --git a/src/sector/bus_removal.rs b/src/sector/bus_removal.rs index e76bd9e6..6db35818 100644 --- a/src/sector/bus_removal.rs +++ b/src/sector/bus_removal.rs @@ -315,6 +315,31 @@ mod tests { ); } + // Per-sector gating WITHIN one multi-sector read at the decorator level: + // content [301,302) covers only the middle sector of a 3-sector read at 300. + // MUTATION: gating the whole buffer on the first sector's LBA goes red here. + #[test] + fn host_key_gates_per_sector_within_a_multi_sector_read() { + let rdk = [0x44u8; 16]; + let clear = clear_content(3); + let mut wire = clear.clone(); + // Encrypt ONLY the middle sector's body; 0 and 2 stay clear on the wire. + let mut mid = clear[SECTOR_BYTES..2 * SECTOR_BYTES].to_vec(); + encrypt_bus(&mut mid, &rdk); + wire[SECTOR_BYTES..2 * SECTOR_BYTES].copy_from_slice(&mid); + + let src = FixedSource { bytes: wire }; + let ranges: Arc<[(u32, u32)]> = Arc::from(vec![(301u32, 1u32)].into_boxed_slice()); + let mut s = BusRemovalSectorSource::new(src, BusStage::AacsHostKey(rdk)) + .with_content_ranges(ranges); + let mut got = vec![0u8; 3 * SECTOR_BYTES]; + s.read_sectors(300, 3, &mut got, false).unwrap(); + assert_eq!( + got, clear, + "only the in-range middle sector is de-bussed; clear neighbours untouched" + ); + } + // Only the reported `n` bytes are de-bussed; a short read must not touch // bytes beyond `n`. Also proves capacity delegates. #[test] diff --git a/src/udf.rs b/src/udf.rs index 6f745b5d..a7dedfd2 100644 --- a/src/udf.rs +++ b/src/udf.rs @@ -4470,6 +4470,124 @@ mod tests { ); } + // read_directory follows a type-3 CONTINUATION pointer (ECMA-167 4/14.14.1.1): + // extent 0 (sector 60) has the first FID; a type-3 AD → block 50 → tail AD → + // extent 1 (sector 61) with the second FID. MUTATION: not following loses it. + #[test] + fn read_directory_follows_type3_continuation() { + let mut e0 = Vec::new(); + push_fid_iu(&mut e0, "", 5, true, true, 0); // parent (..) + push_fid_iu(&mut e0, "FIRST.CLPI", 7, false, false, 1959); + assert_eq!(e0.len(), 2048, "extent 0 must fill its sector exactly"); + let mut sec0 = [0u8; 2048]; + sec0.copy_from_slice(&e0); + + let mut e1 = Vec::new(); + push_fid_iu(&mut e1, "SECOND.CLPI", 8, false, false, 0); + let mut sec1 = [0u8; 2048]; + sec1[..e1.len()].copy_from_slice(&e1); + + // ICB ADs: (recorded, 2048, lba=60) then a type-3 continuation (→ lba 50). + let mut body = Vec::new(); + body.extend_from_slice(&2048u32.to_le_bytes()); + body.extend_from_slice(&60u32.to_le_bytes()); + body.extend_from_slice(&((3u32 << 30) | 2048).to_le_bytes()); + body.extend_from_slice(&50u32.to_le_bytes()); + + let mut reader = MemReader::new(); + reader.put(5, build_dir_icb_flagged(0, &body)); + reader.put(60, sec0); + reader.put(50, build_cont_block(&[(0, e1.len() as u32, 61)])); + reader.put(61, sec1); + reader.put(7, build_efe_icb(11, 2048, 0)); + reader.put(8, build_efe_icb(22, 2048, 0)); + + let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) + .expect("a directory with a continuation AD must be readable"); + assert_eq!( + child_names(&parsed), + vec!["FIRST.CLPI".to_string(), "SECOND.CLPI".to_string()], + "the FID in the continuation block must be read, not dropped" + ); + assert_eq!(parsed.size, 2048 + e1.len() as u64); + } + + // A continuation block opens with a 24-byte AED (tag 258); read_directory reads + // the tail ADs from offset 24, not 0. MUTATION: reading from 0 mis-parses the + // AED tag as a bogus AD and loses the continuation extent (Blu-ray 3D bug). + #[test] + fn read_directory_continuation_skips_aed_header() { + let mut e0 = Vec::new(); + push_fid_iu(&mut e0, "", 5, true, true, 0); + push_fid_iu(&mut e0, "FIRST.CLPI", 7, false, false, 1959); + let mut sec0 = [0u8; 2048]; + sec0.copy_from_slice(&e0); + + let mut e1 = Vec::new(); + push_fid_iu(&mut e1, "SECOND.CLPI", 8, false, false, 0); + let mut sec1 = [0u8; 2048]; + sec1[..e1.len()].copy_from_slice(&e1); + + let mut body = Vec::new(); + body.extend_from_slice(&2048u32.to_le_bytes()); + body.extend_from_slice(&60u32.to_le_bytes()); + body.extend_from_slice(&((3u32 << 30) | 2048).to_le_bytes()); + body.extend_from_slice(&50u32.to_le_bytes()); + + let mut cont = build_cont_block(&[(0, e1.len() as u32, 61)]); + // Stamp a realistic AED tag (258) so a from-offset-0 regression mis-parses it. + cont[0..2].copy_from_slice(&258u16.to_le_bytes()); + + let mut reader = MemReader::new(); + reader.put(5, build_dir_icb_flagged(0, &body)); + reader.put(60, sec0); + reader.put(50, cont); + reader.put(61, sec1); + reader.put(7, build_efe_icb(11, 2048, 0)); + reader.put(8, build_efe_icb(22, 2048, 0)); + + let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) + .expect("continuation with an AED header must be readable"); + assert_eq!( + child_names(&parsed), + vec!["FIRST.CLPI".to_string(), "SECOND.CLPI".to_string()], + "continuation ADs must be read past the 24-byte AED header, not from offset 0" + ); + } + + // Hostile input: a continuation pointer that cycles to its own block. The + // MAX_AD_BLOCKS budget must bound the walk so it TERMINATES (returns the FIDs + // gathered before the cycle) instead of looping forever or panicking. + #[test] + fn read_directory_self_cycling_continuation_terminates() { + let mut e0 = Vec::new(); + push_fid_iu(&mut e0, "", 5, true, true, 0); + push_fid_iu(&mut e0, "FIRST.CLPI", 7, false, false, 1959); + let mut sec0 = [0u8; 2048]; + sec0.copy_from_slice(&e0); + + let mut body = Vec::new(); + body.extend_from_slice(&2048u32.to_le_bytes()); + body.extend_from_slice(&60u32.to_le_bytes()); + body.extend_from_slice(&((3u32 << 30) | 2048).to_le_bytes()); + body.extend_from_slice(&50u32.to_le_bytes()); + + let mut reader = MemReader::new(); + reader.put(5, build_dir_icb_flagged(0, &body)); + reader.put(60, sec0); + // Continuation block 50 points at ITSELF (type-3 → lba 50). + reader.put(50, build_cont_block(&[(3, 2048, 50)])); + reader.put(7, build_efe_icb(11, 2048, 0)); + + let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) + .expect("a self-cycling continuation must terminate, not hang or error"); + assert_eq!( + child_names(&parsed), + vec!["FIRST.CLPI".to_string()], + "FIDs before the cycle are returned; the MAX_AD_BLOCKS budget bounds the walk" + ); + } + #[test] fn read_directory_reads_an_extended_ad_directorys_extent_location() { // ECMA-167 4/14.14.3 extended_ad (20B) keeps extent_location at offset 12, NOT offset 4