fix(tvix/castore): handle Directory::size overflow explicitly

We use checked arithmetic for computing the total size, and verify
that size is in-bounds in Directory::validate.

If an out-of-bounds size makes it to the "unchecked" size method,
we either panic (in debug mode), or silently saturate to u32::MAX.

No new panic sites are added, since overflows in debug mode already
panic at the language level.

Change-Id: I95b8c066a42614fa447f08b4f8fe74e16fbe8bf9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9616
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
This commit is contained in:
edef 2023-10-10 08:55:00 +00:00
parent e2dba089c4
commit baae5ce473
2 changed files with 90 additions and 9 deletions

View file

@ -62,7 +62,7 @@ fn size() {
#[test]
#[cfg_attr(not(debug_assertions), ignore)]
#[should_panic]
#[should_panic = "Directory::size exceeds u32::MAX"]
fn size_unchecked_panic() {
let d = Directory {
directories: vec![DirectoryNode {
@ -78,7 +78,7 @@ fn size_unchecked_panic() {
#[test]
#[cfg_attr(debug_assertions, ignore)]
fn size_unchecked_wrap() {
fn size_unchecked_saturate() {
let d = Directory {
directories: vec![DirectoryNode {
name: "foo".into(),
@ -88,7 +88,53 @@ fn size_unchecked_wrap() {
..Default::default()
};
assert_eq!(d.size(), 0);
assert_eq!(d.size(), u32::MAX);
}
#[test]
fn size_checked() {
// We don't test the overflow cases that rely purely on immediate
// child count, since that would take an absurd amount of memory.
{
let d = Directory {
directories: vec![DirectoryNode {
name: "foo".into(),
digest: DUMMY_DIGEST.to_vec().into(),
size: u32::MAX - 1,
}],
..Default::default()
};
assert_eq!(d.size_checked(), Some(u32::MAX));
}
{
let d = Directory {
directories: vec![DirectoryNode {
name: "foo".into(),
digest: DUMMY_DIGEST.to_vec().into(),
size: u32::MAX,
}],
..Default::default()
};
assert_eq!(d.size_checked(), None);
}
{
let d = Directory {
directories: vec![
DirectoryNode {
name: "foo".into(),
digest: DUMMY_DIGEST.to_vec().into(),
size: u32::MAX / 2,
},
DirectoryNode {
name: "foo".into(),
digest: DUMMY_DIGEST.to_vec().into(),
size: u32::MAX / 2,
},
],
..Default::default()
};
assert_eq!(d.size_checked(), None);
}
}
#[test]
@ -316,3 +362,20 @@ fn validate_sorting() {
d.validate().expect("validate shouldn't error");
}
}
#[test]
fn validate_overflow() {
let d = Directory {
directories: vec![DirectoryNode {
name: "foo".into(),
digest: DUMMY_DIGEST.to_vec().into(),
size: u32::MAX,
}],
..Default::default()
};
match d.validate().expect_err("must fail") {
ValidateDirectoryError::SizeOverflow => {}
_ => panic!("unexpected error"),
}
}