From a018bf9bee2acb934fa4a7b3253df972d5ddf53d Mon Sep 17 00:00:00 2001
From: dzerik <dzerik@gmail.com>
Date: Thu, 6 Aug 2026 09:55:06 +0300
Subject: [PATCH] fix(core): refuse a byte size that does not fit instead of
 wrapping it to zero

`ByteSize::parse` reads the digits, matches the suffix and then multiplies,
and that multiply was unchecked. In a debug build it panics; in a release
build it wraps, which is the case that matters, because it wraps into a
number the caller never wrote and nothing reports it.

`memory = "17179869184G"` is exactly 2^64 bytes. It wrapped to 0 and
installed a ceiling of zero, and a memory ceiling of zero is not inert: the
supervisor registers the handler on `is_some()`, so the guest is SIGKILLed on
its first anonymous mmap while `/proc/meminfo` inside the sandbox reports the
sandbox unlimited. The two readings cannot both stand, and neither of them
names the setting that caused it.

The suffix now resolves to a scale and the multiply is `checked_mul`, so a
value that does not fit is `byte size out of range: <value>`, naming what was
written. The largest value each suffix can carry still parses, so this refuses
what does not fit rather than trimming the usable range.

`ByteSize::kib`, `mib` and `gib` keep their unchecked multiplies. They take a
number the caller already holds rather than text the caller wrote, so the
question they answer is different, and changing their signatures would be a
breaking change to a public type for a case no parse path reaches.
---
 crates/sandlock-core/src/sandbox.rs           | 23 ++++++++++++++-----
 .../tests/integration/test_policy.rs          | 23 +++++++++++++++++++
 2 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs
index a9ed35fd..82a37c29 100644
--- a/crates/sandlock-core/src/sandbox.rs
+++ b/crates/sandlock-core/src/sandbox.rs
@@ -51,12 +51,23 @@ impl ByteSize {
                 .trim()
                 .parse()
                 .map_err(|_| SandboxError::Invalid(format!("invalid byte size: {}", s)))?;
-            match suffix.to_ascii_uppercase().as_str() {
-                "K" => Ok(ByteSize::kib(n)),
-                "M" => Ok(ByteSize::mib(n)),
-                "G" => Ok(ByteSize::gib(n)),
-                other => Err(SandboxError::Invalid(format!("unknown byte size suffix: {}", other))),
-            }
+            let scale: u64 = match suffix.to_ascii_uppercase().as_str() {
+                "K" => 1024,
+                "M" => 1024 * 1024,
+                "G" => 1024 * 1024 * 1024,
+                other => {
+                    return Err(SandboxError::Invalid(format!(
+                        "unknown byte size suffix: {}",
+                        other
+                    )))
+                }
+            };
+            // Checked: the multiply wraps in a release build, so a value that
+            // parses cleanly but does not fit, such as "17179869184G", used to
+            // come back as a ceiling of zero bytes rather than as an error.
+            n.checked_mul(scale)
+                .map(ByteSize)
+                .ok_or_else(|| SandboxError::Invalid(format!("byte size out of range: {}", s)))
         } else {
             let n: u64 = s
                 .parse()
diff --git a/crates/sandlock-core/tests/integration/test_policy.rs b/crates/sandlock-core/tests/integration/test_policy.rs
index c2b0e9b4..02bac3fe 100644
--- a/crates/sandlock-core/tests/integration/test_policy.rs
+++ b/crates/sandlock-core/tests/integration/test_policy.rs
@@ -110,6 +110,29 @@ fn test_bytesize_parse_invalid() {
     assert!(ByteSize::parse("M").is_err());
 }
 
+#[test]
+fn test_bytesize_that_does_not_fit_is_an_error_not_a_ceiling_of_zero() {
+    // The digits parse and the suffix is known, so the value reaches the
+    // multiply. In a release build that multiply wrapped, and every one of
+    // these came back as a byte count the caller never asked for: 17179869184G
+    // is exactly 2^64 bytes, which wrapped to 0 and installed a ceiling of
+    // nothing. A memory ceiling of zero is not inert, the guest is SIGKILLed on
+    // its first allocation, and nothing anywhere named the setting.
+    for spec in ["17179869184G", "17592186044416M", "18014398509481984K"] {
+        let err = ByteSize::parse(spec).expect_err(&format!("{spec} must not parse"));
+        let msg = err.to_string();
+        assert!(
+            msg.contains("out of range") && msg.contains(spec),
+            "{spec} must be refused by name and by reason, got {msg:?}"
+        );
+    }
+
+    // The largest value each suffix can carry still parses, so the check
+    // refuses what does not fit rather than trimming the usable range.
+    assert_eq!(ByteSize::parse("17179869183G").unwrap().0, 17179869183 * 1024 * 1024 * 1024);
+    assert_eq!(ByteSize::parse("18446744073709551615").unwrap().0, u64::MAX);
+}
+
 #[test]
 fn test_clean_env() {
     let p = Sandbox::builder().build().unwrap();