about summary refs log tree commit diff
path: root/tvix/castore/src/proto/tests
diff options
context:
space:
mode:
authoredef <edef@edef.eu>2023-10-10T08·55+0000
committeredef <edef@edef.eu>2023-10-10T20·33+0000
commitbaae5ce473ed83f35f343656eedb14bb60fbecc7 (patch)
treefeb795137ca56d4877389c4f54078615308df64b /tvix/castore/src/proto/tests
parente2dba089c46ae71798d0286f31b207a6b3b66b56 (diff)
fix(tvix/castore): handle Directory::size overflow explicitly r/6777
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
Diffstat (limited to 'tvix/castore/src/proto/tests')
-rw-r--r--tvix/castore/src/proto/tests/directory.rs69
1 files changed, 66 insertions, 3 deletions
diff --git a/tvix/castore/src/proto/tests/directory.rs b/tvix/castore/src/proto/tests/directory.rs
index 688a50f5285e..d46e8cb6c3c4 100644
--- a/tvix/castore/src/proto/tests/directory.rs
+++ b/tvix/castore/src/proto/tests/directory.rs
@@ -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"),
+    }
+}