Make the name-cap tests exercise the planner, not the constants
The round-1 tests asserted arithmetic about MAX_CS0_NAME_BYTES and never called plan(), so both would have passed with the guard deleted — which is the failure mode this audit exists to catch, committed by the audit's own fix. They now build a real folder containing a 255-byte name and require the planner to refuse it, plus a companion proving a name at the cap is still accepted so the guard is not merely refusing everything. The subdirectory cap keeps its arithmetic-only test — creating 65,535 directories is not reasonable in a unit test — but now says so instead of implying coverage it does not have.
This commit is contained in:
+61
-33
@@ -140,7 +140,7 @@ fn fid_len(name: &str, is_parent: bool) -> usize {
|
|||||||
let l_fi = if is_parent {
|
let l_fi = if is_parent {
|
||||||
0
|
0
|
||||||
} else {
|
} else {
|
||||||
super::encode::encode_cs0(name).len()
|
crate::dirimage::encode::encode_cs0(name).len()
|
||||||
};
|
};
|
||||||
(38 + l_fi).div_ceil(4) * 4
|
(38 + l_fi).div_ceil(4) * 4
|
||||||
}
|
}
|
||||||
@@ -204,7 +204,7 @@ fn walk(dir: &Path, disc_path: &str, depth: u32, entries: &mut usize) -> Result<
|
|||||||
// encode something unreadable. 255 ASCII bytes is a legal name on
|
// encode something unreadable. 255 ASCII bytes is a legal name on
|
||||||
// ext4/APFS/NTFS and already exceeds this once the CS0 compression
|
// ext4/APFS/NTFS and already exceeds this once the CS0 compression
|
||||||
// byte is added, so it is reachable without anything exotic.
|
// byte is added, so it is reachable without anything exotic.
|
||||||
if super::encode::encode_cs0(&name).len() > MAX_CS0_NAME_BYTES {
|
if crate::dirimage::encode::encode_cs0(&name).len() > MAX_CS0_NAME_BYTES {
|
||||||
return Err(Error::DirNameTooLong { path: child_path });
|
return Err(Error::DirNameTooLong { path: child_path });
|
||||||
}
|
}
|
||||||
*entries += 1;
|
*entries += 1;
|
||||||
@@ -760,46 +760,74 @@ mod tests {
|
|||||||
assert!(!is_excluded("VTS_01_1.VOB"));
|
assert!(!is_excluded("VTS_01_1.VOB"));
|
||||||
}
|
}
|
||||||
|
|
||||||
/// A name too long for the FID's one-byte length field is refused while
|
/// A name too long for the FID's one-byte length field is refused by the
|
||||||
/// planning.
|
/// PLANNER, on a real folder.
|
||||||
///
|
///
|
||||||
/// Audit finding: the length was narrowed with `as u8`, so a 255-byte ASCII
|
/// Audit finding: the length was narrowed with `as u8`, so a 255-byte ASCII
|
||||||
/// name — legal on ext4/APFS/NTFS — encoded to 256 bytes with the CS0
|
/// name — POSIX NAME_MAX, legal on ext4/APFS/NTFS — encoded to 256 bytes
|
||||||
/// compression byte and wrote a length of ZERO. Every later entry in that
|
/// with the CS0 compression byte and wrote a length of ZERO, making every
|
||||||
/// directory would then be read from the wrong offset, losing files with no
|
/// later entry in that directory read from the wrong offset.
|
||||||
/// error. The cap must sit below the point where the field wraps.
|
///
|
||||||
|
/// An earlier version of this test asserted arithmetic about the constants
|
||||||
|
/// and never called `plan`, so it would have passed with the guard deleted.
|
||||||
#[test]
|
#[test]
|
||||||
fn an_over_long_name_is_refused_not_truncated() {
|
fn an_over_long_name_is_refused_by_the_planner() {
|
||||||
let longest_ok = "a".repeat(MAX_CS0_NAME_BYTES - 1);
|
let dir = std::env::temp_dir().join(format!(
|
||||||
|
"fmkv-longname-{}-{:?}",
|
||||||
|
std::process::id(),
|
||||||
|
std::time::SystemTime::now()
|
||||||
|
.duration_since(std::time::UNIX_EPOCH)
|
||||||
|
.unwrap()
|
||||||
|
.as_nanos()
|
||||||
|
));
|
||||||
|
std::fs::create_dir_all(dir.join("BDMV/STREAM")).expect("mkdir");
|
||||||
|
// 255 bytes: the exact length that used to narrow to zero.
|
||||||
|
let name = "a".repeat(255);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
super::super::encode::encode_cs0(&longest_ok).len(),
|
crate::dirimage::encode::encode_cs0(&name).len(),
|
||||||
MAX_CS0_NAME_BYTES,
|
256,
|
||||||
"fixture: the longest accepted name encodes to exactly the cap"
|
"fixture: NAME_MAX encodes to 256 bytes with the compression byte"
|
||||||
);
|
);
|
||||||
|
std::fs::write(dir.join("BDMV/STREAM").join(&name), b"x").expect("write");
|
||||||
|
|
||||||
|
let err = plan(&dir).expect_err("the planner must refuse this folder");
|
||||||
assert!(
|
assert!(
|
||||||
MAX_CS0_NAME_BYTES < u8::MAX as usize,
|
matches!(err, Error::DirNameTooLong { .. }),
|
||||||
"the cap must leave the length field unable to wrap"
|
"expected DirNameTooLong, got {err:?}"
|
||||||
);
|
|
||||||
// The exact case that used to write a length of zero: a 255-byte name
|
|
||||||
// (the POSIX NAME_MAX, so entirely ordinary) encodes to 256 bytes once
|
|
||||||
// the CS0 compression byte is prepended, and 256 narrows to 0 in a u8.
|
|
||||||
let name_max = "a".repeat(255);
|
|
||||||
let encoded = super::super::encode::encode_cs0(&name_max).len();
|
|
||||||
assert_eq!(encoded, 256, "fixture: NAME_MAX encodes to 256 bytes");
|
|
||||||
assert_eq!(
|
|
||||||
encoded as u8, 0,
|
|
||||||
"fixture: this is the narrowing that silently zeroed the field"
|
|
||||||
);
|
|
||||||
assert!(
|
|
||||||
encoded > MAX_CS0_NAME_BYTES,
|
|
||||||
"so the planner must refuse it before the encoder sees it"
|
|
||||||
);
|
);
|
||||||
|
let _ = std::fs::remove_dir_all(&dir);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The subdirectory cap keeps a directory's 16-bit link count from wrapping.
|
/// A name at the cap is accepted, so the guard rejects only what it must.
|
||||||
///
|
#[test]
|
||||||
/// Audit finding: the count was `1 + dirs.len() as u16`, and the global
|
fn a_name_at_the_cap_is_accepted() {
|
||||||
/// entry cap alone permits one directory holding 65,535 subdirectories.
|
let dir = std::env::temp_dir().join(format!(
|
||||||
|
"fmkv-okname-{}-{:?}",
|
||||||
|
std::process::id(),
|
||||||
|
std::time::SystemTime::now()
|
||||||
|
.duration_since(std::time::UNIX_EPOCH)
|
||||||
|
.unwrap()
|
||||||
|
.as_nanos()
|
||||||
|
));
|
||||||
|
std::fs::create_dir_all(dir.join("BDMV/STREAM")).expect("mkdir");
|
||||||
|
// One byte of name shorter, so the encoding lands exactly on the cap.
|
||||||
|
let name = "a".repeat(MAX_CS0_NAME_BYTES - 1);
|
||||||
|
assert_eq!(
|
||||||
|
crate::dirimage::encode::encode_cs0(&name).len(),
|
||||||
|
MAX_CS0_NAME_BYTES
|
||||||
|
);
|
||||||
|
std::fs::write(dir.join("BDMV/STREAM").join(&name), b"x").expect("write");
|
||||||
|
assert!(
|
||||||
|
plan(&dir).is_ok(),
|
||||||
|
"a name whose encoding equals the cap must be accepted"
|
||||||
|
);
|
||||||
|
let _ = std::fs::remove_dir_all(&dir);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The subdirectory cap must keep a directory's 16-bit link count
|
||||||
|
/// representable. Creating 65,535 directories in a test is not reasonable,
|
||||||
|
/// so this pins the arithmetic relationship the guard relies on — and says
|
||||||
|
/// plainly that it does NOT exercise `walk`.
|
||||||
#[test]
|
#[test]
|
||||||
fn the_subdir_cap_keeps_the_link_count_representable() {
|
fn the_subdir_cap_keeps_the_link_count_representable() {
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
|
|||||||
Reference in New Issue
Block a user