From 26423187d3c1c9fb618d4947111690d515b98f12 Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Thu, 9 Jul 2026 14:41:49 -0700 Subject: [PATCH] audit: bound the VTI clip-table scan; fix stale aacs doc links MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 findings from the 10-phase release audit: - parse_vti_clip_order bucketed hits by residue with an O(stride*hits) rescan and no hit cap, so a crafted HD-DVD VTI packed with millions of `.EVO` tokens (up to the 64 MiB UDF read cap) could burn seconds of CPU on a routine scan. Bucket in a single O(hits) pass and cap collected hits at MAX_VTI_HITS (a real table holds a few dozen). - Fix the stale `super::keys::…` intra-doc links left by the aacs module rename: the referenced fns live in `super::derive`. --- src/aacs/crypto.rs | 2 +- src/aacs/variant.rs | 8 ++++---- src/disc/hddvd.rs | 36 +++++++++++++++++------------------- 3 files changed, 22 insertions(+), 24 deletions(-) diff --git a/src/aacs/crypto.rs b/src/aacs/crypto.rs index 7ebd407..fcf2a7f 100644 --- a/src/aacs/crypto.rs +++ b/src/aacs/crypto.rs @@ -69,7 +69,7 @@ pub(crate) fn aes_cbc_decrypt(key: &[u8; 16], data: &mut [u8]) { /// /// The Media Key Variant chain uses AES-G to derive both the variant /// number (`Kvn = AES-G(Kp, Nonce)`) and the Volume Unique Key -/// (`Kvu = AES-G(Km, VID)`). See [`super::keys::derive_vuk`] for the +/// (`Kvu = AES-G(Km, VID)`). See [`super::derive::derive_vuk`] for the /// classical VUK form — the math is identical, this exposes it as a /// neutral primitive for the variant chain. pub(crate) fn aes_g(x1: &[u8; 16], x2: &[u8; 16]) -> [u8; 16] { diff --git a/src/aacs/variant.rs b/src/aacs/variant.rs index 7f4dde8..bc4e99d 100644 --- a/src/aacs/variant.rs +++ b/src/aacs/variant.rs @@ -138,7 +138,7 @@ pub(crate) fn variant_key_data(records: &[MkbRecord]) -> Option<&[u8]> { // ── Subset-difference walk that exposes (Kp, uv) ────────────────────────── // `calc_v_mask` and `calc_pk_from_dk` (and the AES-G3 seed step they ride -// on) are shared with the classical walk in [`super::keys`] — a single +// on) are shared with the classical walk in [`super::derive`] — a single // definition keeps the variant SD tree byte-identical to the classical one. // (`aesg3` itself is imported separately in the test module.) use super::derive::{calc_pk_from_dk, calc_v_mask}; @@ -172,7 +172,7 @@ fn mkb_find_mk_dv(records: &[MkbRecord]) -> Option<[u8; 16]> { /// `device_keys` covers. Returns `None` if no DK walks any uv. /// /// This is the AACS-2.1 **variant** walk; the classical walk lives in -/// [`super::keys::derive_media_key_and_pk_from_dk`]. The two are kept +/// [`super::derive::derive_media_key_and_pk_from_dk`]. The two are kept /// separate on purpose and select MKB records in DELIBERATELY different /// order: /// @@ -181,7 +181,7 @@ fn mkb_find_mk_dv(records: &[MkbRecord]) -> Option<[u8; 16]> { /// small `0x07` Explicit-Subset-Difference record carries the /// cvalue the Precursor chain consumes, whereas a classical UHD MKB /// keeps its 1:1 cvalue table in the large `0x05` record (see the -/// note on [`super::keys::probe::mkb_cvalues`]). They must NOT be +/// note on [`super::derive::probe::mkb_cvalues`]). They must NOT be /// unified to one order — each is correct for its own MKB shape. /// - finders: this walk operates on parsed [`MkbRecord`]s (needed /// because the variant chain also reads `0x2d`/`0x2f`); the @@ -629,7 +629,7 @@ pub fn media_key_variant_from_kp( #[cfg(test)] mod tests { use super::*; - // These three live in `super::keys` now (consolidated SD-walk helpers); + // These three live in `super::derive` now (consolidated SD-walk helpers); // `use super::*` does not re-export the parent module's private `use` // imports, so pull them in directly for the tests below. use super::super::crypto::aesg3; diff --git a/src/disc/hddvd.rs b/src/disc/hddvd.rs index b1074c5..4921e63 100644 --- a/src/disc/hddvd.rs +++ b/src/disc/hddvd.rs @@ -73,11 +73,18 @@ fn parse_vti_clip_order(vti: &[u8]) -> Vec { if !vti.starts_with(HDDVD_VTI_MAGIC) { return Vec::new(); } - // Collect (offset, name) for every NUL-terminated printable run ending `.EVO`. + // A real VTI clip table holds a few dozen entries; cap the collected hits so + // a crafted VTI packed with millions of `.EVO` tokens (up to the 64 MiB UDF + // read cap) can't burn CPU or memory during a routine scan. + const MAX_VTI_HITS: usize = 8192; let is_name_byte = |b: u8| b.is_ascii_graphic(); - let mut hits: Vec<(usize, String)> = Vec::new(); + // Bucket hits by residue-mod-stride in a SINGLE pass — the clip table shares + // one residue, so the largest bucket is it (avoids an O(stride*hits) rescan). + let mut buckets: std::collections::HashMap> = + std::collections::HashMap::new(); + let mut count = 0usize; let mut i = 0usize; - while i < vti.len() { + while i < vti.len() && count < MAX_VTI_HITS { if !is_name_byte(vti[i]) { i += 1; continue; @@ -90,25 +97,16 @@ fn parse_vti_clip_order(vti: &[u8]) -> Vec { let nul_terminated = i < vti.len() && vti[i] == 0; if nul_terminated && name.len() >= 5 && name[name.len() - 4..].eq_ignore_ascii_case(b".EVO") { - hits.push((start, String::from_utf8_lossy(name).into_owned())); + buckets + .entry(start % VTI_CLIP_ENTRY_STRIDE) + .or_default() + .push((start, String::from_utf8_lossy(name).into_owned())); + count += 1; } } - if hits.is_empty() { + let Some(mut best) = buckets.into_values().max_by_key(|g| g.len()) else { return Vec::new(); - } - // The clip-table entries all share one residue mod stride; other stray `.EVO` - // references (if any) fall in different residues. Keep the largest group. - let mut best: Vec<(usize, String)> = Vec::new(); - for res in 0..VTI_CLIP_ENTRY_STRIDE { - let group: Vec<(usize, String)> = hits - .iter() - .filter(|(o, _)| o % VTI_CLIP_ENTRY_STRIDE == res) - .cloned() - .collect(); - if group.len() > best.len() { - best = group; - } - } + }; best.sort_by_key(|(o, _)| *o); best.into_iter().map(|(_, n)| n).collect() }