From 2a450b86cb4f1a115e56cef613a2db1c38f1ae82 Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Mon, 19 Aug 2024 12:43:08 -0400
Subject: [PATCH] rust: Also test we fail to parse the `should-fail-` cases

This way, callers of the Rust API get validation before
we start passing things down to the C code.
(To be clear, the C code also needs sanity tests, but
 it's way more reliable to test things on the Rust side)

Signed-off-by: Colin Walters <walters@verbum.org>
---
 rust/composefs/src/dumpfile.rs | 66 ++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/rust/composefs/src/dumpfile.rs b/rust/composefs/src/dumpfile.rs
index c0e99d96..d9baf288 100644
--- a/rust/composefs/src/dumpfile.rs
+++ b/rust/composefs/src/dumpfile.rs
@@ -16,6 +16,13 @@ use std::str::FromStr;
 use anyhow::Context;
 use anyhow::{anyhow, Result};
 
+/// https://github.com/torvalds/linux/blob/47ac09b91befbb6a235ab620c32af719f8208399/include/uapi/linux/limits.h#L15
+/// This isn't exposed in libc/rustix, and in any case we should be conservative...if this ever
+/// gets bumped it'd be a hazard.
+const XATTR_NAME_MAX: usize = 255;
+// See above
+const XATTR_SIZE_MAX: usize = u16::MAX as usize;
+
 #[derive(Debug, PartialEq, Eq)]
 /// An extended attribute entry
 pub struct Xattr<'k> {
@@ -222,7 +229,23 @@ impl<'k> Xattr<'k> {
             .split_once('=')
             .ok_or_else(|| anyhow!("Missing = in xattrs"))?;
         let key = unescape_to_osstr(key)?;
+        let keylen = key.as_bytes().len();
+        if keylen > XATTR_NAME_MAX {
+            anyhow::bail!(
+                "xattr name too long; max={} found={}",
+                XATTR_NAME_MAX,
+                keylen
+            );
+        }
         let value = unescape(value)?;
+        let valuelen = value.len();
+        if valuelen > XATTR_SIZE_MAX {
+            anyhow::bail!(
+                "xattr value too long; max={} found={}",
+                XATTR_SIZE_MAX,
+                keylen
+            );
+        }
         Ok(Self { key, value })
     }
 }
@@ -264,6 +287,10 @@ impl<'p> Entry<'p> {
                 libc::S_IFLNK => {
                     let target =
                         unescape_to_path(payload.ok_or_else(|| anyhow!("Missing payload"))?)?;
+                    let targetlen = target.as_os_str().as_bytes().len();
+                    if targetlen > libc::PATH_MAX as usize {
+                        anyhow::bail!("Target length too large {}", targetlen);
+                    }
                     Item::Symlink { nlink, target }
                 }
                 libc::S_IFIFO => Item::Fifo { nlink },
@@ -445,4 +472,43 @@ mod tests {
             assert_eq!(e, e2);
         }
     }
+
+    fn parse_all(name: &str, s: &str) -> Result<()> {
+        for line in s.lines() {
+            if line.is_empty() {
+                continue;
+            }
+            let _: Entry =
+                Entry::parse(line).with_context(|| format!("Test case={name:?} line={line:?}"))?;
+        }
+        Ok(())
+    }
+
+    #[test]
+    fn test_should_fail() {
+        const CASES: &[(&str, &str)] = &[
+            (
+                "long link",
+                include_str!("../../../tests/assets/should-fail-long-link.dump"),
+            ),
+            (
+                "no ftype",
+                include_str!("../../../tests/assets/should-fail-no-ftype.dump"),
+            ),
+            (
+                "long xattr key",
+                include_str!("../../../tests/assets/should-fail-long-xattr-key.dump"),
+            ),
+            (
+                "long xattr value",
+                include_str!("../../../tests/assets/should-fail-long-xattr-value.dump"),
+            ),
+        ];
+        for (name, case) in CASES.iter().copied() {
+            assert!(
+                parse_all(name, case).is_err(),
+                "Expected case {name} to fail"
+            );
+        }
+    }
 }