Stop an uncrackable VTS borrowing another VTS's title key

`resolve_vts_key` called the `Option`-returning `css::crack_key`, which
collapses "no scrambled sector was seen" with "scrambled sectors were
seen and no key came out", and then fell back to the disc-wide key. A
multi-VTS CSS DVD whose second title set resists the Stevenson scan was
descrambled under the FIRST set's key: corrupt PES behind an intact
header, written out with `complete = true` at exit 0.

`CrackOutcome` exists precisely to keep those two apart, and its doc says
callers must surface the second as a hard error. Every sibling path in
the crate already does — `Disc::decrypt_keys_for_title` and the mux path
both map `ScrambledUncracked` to `CssKeyMissing`. This was the one path
that did not. The ordering comment 15 lines above describes this exact
outcome as the bug it exists to prevent; ordering makes the crack far
more likely to succeed, but it cannot make a failed crack safe.

Also, three things nothing could catch:

- The demux output filename took the stream's language raw while the
  base beside it was sanitised. A language code is three raw STN bytes
  through `from_utf8_lossy`, and `00 00 00` is the ordinary "undefined"
  encoding on real discs — a NUL in a path fails `File::create` with
  InvalidInput, taking the whole export down before one track file
  opened. `sanitize` now maps control characters too; it did not.

- `css::crack_key_scan`'s short-read handling was dead code under test:
  every source in the module returned the full request, so reverting
  `advance` to the requested count, or dropping the `.max(1)`, left the
  suite green. The `.max(1)` is load-bearing — without it a source
  returning `Ok(0)` never moves the cursor and never increments the
  budget, so the scan spins forever. That mutation now HANGS the test
  rather than failing it, which is the honest demonstration.

- `MAX_SUBDIRS`'s const-assert carried `#[cfg(not(test))]` inside a
  `#[cfg(test)] mod tests`, so it compiled in no configuration and could
  never fire — the dead gate the test above it was written to replace.
  Moved to module scope, and verified it now rejects a wrong constant at
  compile time.

src/css/mod.rs

@@ -738,6 +738,11 @@ mod tests {
         /// instead of the uniform `flag_byte` fill. Lets the scan actually
         /// reach `CrackOutcome::Cracked` from a synthetic ISO.
         crackable: Option<(u32, Vec<u8>)>,
+        /// Sectors actually filled per batch, however many were asked for —
+        /// the SHORT READ a `recovery: true` source is allowed to return over
+        /// a damaged region. `Some(0)` is the degenerate case that must not
+        /// spin the scan.
+        short_read: Option<usize>,
     }
 
     impl MockSource {
@@ -748,6 +753,7 @@ mod tests {
                 fail_all: false,
                 lock_all: false,
                 crackable: None,
+                short_read: None,
             }
         }
     }
@@ -798,14 +804,19 @@ mod tests {
             if self.fail_all {
                 return Err(Error::DecryptFailed);
             }
-            let n = count as usize * 2048;
+            // A short read fills, and reports, fewer sectors than asked.
+            let filled = match self.short_read {
+                Some(k) => (k as u16).min(count),
+                None => count,
+            };
+            let n = filled as usize * 2048;
             let end = n.min(buf.len());
             for b in buf[..end].iter_mut() {
                 *b = 0;
             }
             // Fill each sector in the batch with the uniform flag byte, EXCEPT a
             // designated crackable LBA which gets the full synthetic sector.
-            for s in 0..count as u32 {
+            for s in 0..filled as u32 {
                 let sect_lba = lba + s;
                 let base = s as usize * 2048;
                 if base + 2048 > end {
@@ -830,6 +841,60 @@ mod tests {
         }
     }
 
+    // ── Short reads: the branch nothing exercised ─────────────────────────
+    //
+    // `crack_key_scan` passes `recovery = true`, which is precisely the mode
+    // where a `SectorSource` may return Ok with fewer bytes than asked. Every
+    // source in this module returned the full request, so `usable`, `advance`
+    // and the `.max(1)` anti-spin guard were dead code under test: reverting
+    // `advance` to `n`, or dropping the `.max(1)`, left the whole suite green.
+
+    /// A short batch is RE-READ from where it stopped, not skipped. Skipping
+    /// would quietly shrink the crack's coverage on exactly the damaged media
+    /// where a key is hardest to find.
+    #[test]
+    fn a_short_read_resumes_from_where_it_stopped() {
+        let mut src = MockSource::new(0x00);
+        src.short_read = Some(1);
+        let ext = [crate::disc::Extent {
+            start_lba: 100,
+            sector_count: 4,
+        }];
+        let _ = crack_key_scan(&mut src, &ext, 4, None, false);
+        let reads = src.reads.borrow().clone();
+        assert_eq!(
+            reads,
+            vec![100, 101, 102, 103],
+            "a source that filled one sector per batch must be asked for the \
+             next one, not advanced a whole batch past it"
+        );
+    }
+
+    /// A source that reads NOTHING must terminate. Without the `.max(1)` the
+    /// cursor never moves and `tried` never increments — the budget cannot end
+    /// the loop, so the scan spins forever inside a library whose whole job is
+    /// surviving hostile input.
+    #[test]
+    fn a_source_that_returns_zero_sectors_terminates() {
+        let mut src = MockSource::new(0x00);
+        src.short_read = Some(0);
+        let ext = [crate::disc::Extent {
+            start_lba: 0,
+            sector_count: 8,
+        }];
+        let outcome = crack_key_scan(&mut src, &ext, 4, None, false);
+        assert!(
+            matches!(outcome, CrackOutcome::Unencrypted),
+            "nothing was read, so nothing scrambled was seen"
+        );
+        assert!(
+            src.reads.borrow().len() <= 8,
+            "the cursor must advance even on an empty read; got {} reads over \
+             an 8-sector extent",
+            src.reads.borrow().len()
+        );
+    }
+
     /// crack_key caps total scanned sectors at 50_000 even when extents are
     /// far larger, and counts EVERY scanned sector (clear ones included)
     /// toward the budget. With one 200_000-sector extent of clear sectors, it

src/dirimage/layout.rs

@@ -67,6 +67,15 @@ const MAX_SUBDIRS: usize = (u16::MAX - 1) as usize;
 #[cfg(test)]
 const MAX_SUBDIRS: usize = 4;
 
+/// One entry per child plus the parent's own must still fit the 16-bit field.
+///
+/// At MODULE scope, and it has to be. This assertion previously sat inside
+/// `mod tests`, which is `#[cfg(test)]`, while carrying `#[cfg(not(test))]`
+/// itself — so it was compiled in NO configuration and could never fire, which
+/// made it exactly the dead gate the test above it was written to replace.
+#[cfg(not(test))]
+const _: () = assert!(MAX_SUBDIRS + 1 == u16::MAX as usize);
+
 /// Largest image this planner will synthesize, in sectors (128 GiB).
 ///
 /// A DVD title set records where its VOBS begins as an offset in its own IFO,
@@ -998,11 +1007,6 @@ mod tests {
             "expected DirImageFanout, got {err:?}"
         );
 
-        // And the real production value is what ships: one per child plus the
-        // parent's own entry must still fit in the 16-bit field.
-        #[cfg(not(test))]
-        const _: () = assert!(MAX_SUBDIRS + 1 == u16::MAX as usize);
-
         let _ = std::fs::remove_dir_all(&dir);
     }
 }

src/disc/extract.rs

@@ -254,7 +254,7 @@ impl Disc {
                     let key = match vts_keys.get(&vts) {
                         Some(k) => k.clone(),
                         None => {
-                            let k = self.resolve_vts_key(&vts, &planned, &mut dec, &base_keys);
+                            let k = self.resolve_vts_key(&vts, &planned, &mut dec, &base_keys)?;
                             vts_keys.insert(vts.clone(), k.clone());
                             k
                         }
@@ -291,13 +291,24 @@ impl Disc {
     /// VOB extents (keyless Stevenson attack). The disc-wide key is reused when
     /// it already covers this VTS (single-VTS discs, or this VTS's span). The
     /// reader is borrowed from the decrypting decorator (its inner source).
+    ///
+    /// FALLIBLE, because the three outcomes of a crack are not two.
+    /// [`crate::css::CrackOutcome`] exists precisely to separate "no scrambled
+    /// sector was seen, so there is nothing to decrypt" from "scrambled sectors
+    /// were seen and no key came out", and its doc says callers MUST surface
+    /// the second as a hard error. This function used the `Option`-returning
+    /// `crack_key`, which collapses both into `None`, and then fell back to the
+    /// disc-wide key — descrambling this VTS with ANOTHER VTS's title key. The
+    /// hazard is described in the ordering note below, which was written about
+    /// the same fallback; ordering makes the crack far more likely to succeed,
+    /// but it cannot make a failed crack safe.
     fn resolve_vts_key<S: SectorSource>(
         &self,
         vts: &str,
         planned: &[PlannedFile],
         dec: &mut DecryptingSectorSource<S>,
         base_keys: &DecryptKeys,
-    ) -> DecryptKeys {
+    ) -> Result<DecryptKeys> {
         // Gather the title VOB extents for this VTS (VTS_xx_1.VOB .. _9.VOB;
         // VTS_xx_0.VOB is the menu and is clear, so excluded from the crack).
         // Gather this VTS's title VOBs BY NAME, ascending.
@@ -336,7 +347,7 @@ impl Disc {
             }
         }
         if extents.is_empty() {
-            return base_keys.clone();
+            return Ok(base_keys.clone());
         }
         // PLAYBACK ORDER — do NOT sort. This is the 1.5.1 garbage bug, and it
         // grew back here in a new code path: the comment this replaced said it
@@ -358,11 +369,20 @@ impl Disc {
         // in file order, which together is playback order for a DVD title set.
         // Crack against the raw (still-scrambled) inner reader, NOT the
         // decrypting view — `crack_key` runs the descrambler itself.
-        match crate::css::crack_key(dec.inner_mut(), &extents, 64) {
+        match crate::css::crack_key_outcome(dec.inner_mut(), &extents, 64, None) {
-            Some(state) => DecryptKeys::Css {
+            crate::css::CrackOutcome::Cracked(state) => Ok(DecryptKeys::Css {
                 title_key: state.title_key,
-            },
+            }),
-            None => base_keys.clone(),
+            // No scrambled sector anywhere in this VTS: the content is clear,
+            // and any key descrambles it as a no-op. The disc-wide key is the
+            // right answer, and this is the ONLY case that ever was.
+            crate::css::CrackOutcome::Unencrypted => Ok(base_keys.clone()),
+            // Scrambled sectors WERE seen and no key came out. Reusing the
+            // disc-wide key here writes corrupt PES behind an intact header and
+            // reports a complete extract at exit 0. Skippable per title, which
+            // is why this is the per-title code and not the disc-level one — a
+            // sibling VTS may still crack.
+            crate::css::CrackOutcome::ScrambledUncracked => Err(Error::CssKeyMissing),
         }
     }
 }
@@ -1918,6 +1938,87 @@ mod tests {
         assert!(res.complete);
     }
 
+    /// A VTS that IS scrambled but whose key could not be recovered must
+    /// FAIL, not borrow another VTS's key.
+    ///
+    /// `crack_key` returns `Option`, which collapses "no scrambled sector was
+    /// seen" with "scrambled sectors were seen and no key came out"; the
+    /// fallback then descrambled this VTS under the disc-wide key and
+    /// `extract_tree` reported `complete = true` at exit 0, because no read had
+    /// failed. `CrackOutcome` exists to keep those two apart, and every sibling
+    /// path in the crate already honours it.
+    ///
+    /// The fixture: VTS_01 is crackable, VTS_02 is scrambled with NO periodic
+    /// crib, so its crack genuinely fails.
+    #[test]
+    fn a_scrambled_vts_that_cannot_be_cracked_fails_instead_of_borrowing_a_key() {
+        let key_1 = [0x10u8, 0x20, 0x30, 0x40, 0x50];
+        let (_plain_1, scrambled_1) = {
+            let seed = [0x11u8, 0x22, 0x33, 0x44, 0x01];
+            let mut plain = vec![0u8; 2048];
+            plain[0x00..0x04].copy_from_slice(&[0x00, 0x00, 0x01, 0xBA]);
+            plain[0x14] = 0x10;
+            let pat: Vec<u8> = (0..8)
+                .map(|k| (0xA0u8.wrapping_add(k as u8) ^ 0x01) ^ 0x5A)
+                .collect();
+            for (i, b) in plain.iter_mut().enumerate().skip(0x59) {
+                *b = pat[i % 8];
+            }
+            plain[0x54..0x59].copy_from_slice(&seed);
+            let mut scrambled = plain.clone();
+            lfsr::scramble_sector(&key_1, &mut scrambled);
+            (plain, scrambled)
+        };
+        // Scrambled — the pack header and the 0x14 flag are set, so the scan
+        // SEES ciphertext — but the cleartext region carries no periodic run,
+        // so the Stevenson crib never forms and no key is recoverable.
+        let uncrackable = {
+            let mut sect = vec![0u8; 2048];
+            sect[0x00..0x04].copy_from_slice(&[0x00, 0x00, 0x01, 0xBA]);
+            sect[0x14] = 0x10;
+            for (i, b) in sect.iter_mut().enumerate().skip(0x59) {
+                // Non-repeating, so no run of any period survives to 0x80.
+                *b = (i as u8).wrapping_mul(37).wrapping_add(11);
+            }
+            sect
+        };
+
+        let root = DirSpec {
+            name: String::new(),
+            icb_lba: 10,
+            dir_data_lba: 11,
+            files: Vec::new(),
+            subdirs: vec![DirSpec {
+                name: "VIDEO_TS".to_string(),
+                icb_lba: 20,
+                dir_data_lba: 21,
+                files: vec![
+                    file("VTS_01_1.VOB", 30, 5000, scrambled_1, false),
+                    file("VTS_02_1.VOB", 32, 6000, uncrackable, false),
+                ],
+                subdirs: vec![],
+            }],
+        };
+        let mut disc = build_disc(root);
+        let out = TmpDir::new("css_uncrackable_vts");
+        let mut d = clear_disc();
+        d.content_format = crate::disc::ContentFormat::MpegPs;
+        d.css = Some(crate::css::CssState {
+            title_key: [0xFFu8; 5],
+            crack_span: None,
+        });
+        let err = d
+            .extract_tree(&mut disc, out.path(), &ExtractOptions::default())
+            .expect_err(
+                "a VTS whose key could not be recovered must be a hard error, \
+                 not a silent extract under another VTS's key",
+            );
+        assert!(
+            matches!(err, Error::CssKeyMissing),
+            "expected CssKeyMissing, got {err:?}"
+        );
+    }
+
     /// `Borrowed` is a thin forwarding wrapper the decrypting decorator uses
     /// to avoid taking ownership of the caller's reader -- every
     /// `SectorSource` method must forward to the wrapped `&mut dyn

src/mux/demux_sink.rs

@@ -637,10 +637,18 @@ fn xml_escape(s: &str) -> String {
 }
 
 /// Replace path-hostile characters in a filename component.
+///
+/// Control characters included, NUL above all. Every string this touches is
+/// disc bytes, and a language code is three raw STN bytes run through
+/// `from_utf8_lossy` with no validation — `00 00 00` is the ordinary
+/// "undefined" encoding on real Blu-rays. A NUL in a path aborts `File::create`
+/// with `InvalidInput`, which took the whole demux export down before a single
+/// track file was opened.
 fn sanitize(s: &str) -> String {
     s.chars()
         .map(|c| match c {
             '/' | '\\' | ':' | '*' | '?' | '"' | '<' | '>' | '|' => '_',
+            c if c.is_control() => '_',
             _ => c,
         })
         .collect()
@@ -787,8 +795,11 @@ impl DemuxSink {
             Naming::Pid => format!("{} {:04x}", sanitize(&opts.base), pid),
             Naming::Friendly => {
                 let mut parts = vec![sanitize(&opts.base), format!("t{idx:02}")];
+                // The language is disc bytes too, and got none of the
+                // treatment `opts.base` two lines up already had.
+                let lang = sanitize(lang);
                 if !lang.is_empty() {
-                    parts.push(lang.to_string());
+                    parts.push(lang);
                 }
                 parts.push(codec_label(codec).to_string());
                 parts.join(" ")
@@ -987,6 +998,52 @@ mod tests {
         Resolution, SampleRate, VideoStream,
     };
 
+    // ── The language component is disc bytes ──────────────────────────────
+    //
+    // `opts.base` was sanitised and the language beside it was not, though a
+    // language code is three raw STN bytes run through `from_utf8_lossy` with
+    // no validation. `00 00 00` is the ordinary "undefined" encoding on real
+    // Blu-rays, and a NUL in a path fails `File::create` with `InvalidInput` —
+    // which aborted the whole export before a single track file was opened.
+
+    /// The stem stays one usable filename component whatever the disc says.
+    #[test]
+    fn a_hostile_language_code_cannot_break_the_output_filename() {
+        let opts = DemuxOptions {
+            base: "Movie".to_string(),
+            ..Default::default()
+        };
+        assert!(matches!(opts.naming, Naming::Friendly), "default naming");
+
+        for lang in ["\u{0}\u{0}\u{0}", "a/b", "..", "a\nb", "e:s"] {
+            let stem = DemuxSink::stem_for(&opts, 1, 0x1100, lang, Codec::Ac3);
+            assert!(
+                !stem.chars().any(|c| c.is_control()),
+                "control character survived into the filename for {lang:?}: {stem:?}"
+            );
+            assert!(
+                !stem.contains('/') && !stem.contains('\\') && !stem.contains(':'),
+                "a path separator survived for {lang:?}: {stem:?}"
+            );
+            assert!(
+                std::path::Path::new(&stem).components().count() == 1,
+                "the stem must stay ONE component for {lang:?}: {stem:?}"
+            );
+        }
+    }
+
+    /// A legitimate language is still carried through untouched — the
+    /// sanitiser must not be so lossy that it stops naming the track.
+    #[test]
+    fn an_ordinary_language_code_survives_sanitising() {
+        let opts = DemuxOptions {
+            base: "Movie".to_string(),
+            ..Default::default()
+        };
+        let stem = DemuxSink::stem_for(&opts, 1, 0x1100, "eng", Codec::Ac3);
+        assert!(stem.contains("eng"), "got {stem:?}");
+    }
+
     fn video_stream(codec: Codec) -> DiscStream {
         DiscStream::Video(VideoStream {
             pid: 0x1011,