security: do not follow directory symlinks in cargo xtask stats (CWE-400) (#1845)

* security: do not follow directory symlinks in `cargo xtask stats` (CWE-400)

The stats walker descended with `Path::is_dir()`, which follows symlinks, and
kept no record of visited directories. A small acyclic graph of directory
aliases planted in a workspace package (for example through a contribution a
developer inspects) therefore caused exponentially repeated enumeration and file
reads, consuming CPU and I/O far beyond the repository's real size.

`walk` now inspects each entry's own `file_type()` and skips symlinks before
descending or reading, so directory aliases are never re-scanned. A traversal
budget (`MAX_ENTRIES`) bounds total work as defense in depth.

Addresses finding row 4.

* fix: report statistics traversal exhaustion

---------

Co-authored-by: skylartaylor <26588250+skylartaylor@users.noreply.github.com>

xtask/src/stats.rs

@@ -10,20 +10,41 @@ struct Counts {
     proptests: usize,
 }
 
-fn walk(dir: &Path, c: &mut Counts) {
+/// Upper bound on directory entries visited in one `walk`, so even a crafted tree of aliases can
+/// never make the stats command run unbounded work.
+const MAX_ENTRIES: usize = 2_000_000;
+
+fn walk(dir: &Path, c: &mut Counts) -> Result<(), String> {
+    let mut budget = MAX_ENTRIES;
+    walk_inner(dir, c, &mut budget)
+}
+
+fn walk_inner(dir: &Path, c: &mut Counts, budget: &mut usize) -> Result<(), String> {
     let Ok(rd) = std::fs::read_dir(dir) else {
-        return;
+        return Ok(());
     };
     for e in rd.flatten() {
+        if *budget == 0 {
+            return Err(format!("stats traversal budget exhausted at {}", dir.display()));
+        }
+        *budget -= 1;
+        // Use the entry's own type and never follow symlinks: a directory symlink would otherwise
+        // be re-scanned as a fresh directory, so a small graph of aliases could cause exponential
+        // repeated enumeration and reads (CWE-400).
+        let Ok(ft) = e.file_type() else { continue };
+        if ft.is_symlink() {
+            continue;
+        }
         let p = e.path();
         let name = e.file_name();
         let name = name.to_string_lossy();
-        if p.is_dir() {
+        if ft.is_dir() {
             // Nested fuzz workspaces and build output are not part of the crate.
             if name != "target" && name != "fuzz" && !name.starts_with('.') {
-                walk(&p, c);
+                walk_inner(&p, c, budget)?;
             }
-        } else if name.ends_with(".rs")
+        } else if ft.is_file()
+            && name.ends_with(".rs")
             && let Ok(s) = std::fs::read_to_string(&p)
         {
             c.files += 1;
@@ -33,6 +54,7 @@ fn walk(dir: &Path, c: &mut Counts) {
             c.proptests += pt;
         }
     }
+    Ok(())
 }
 
 /// `#[test]` attributes and `proptest!` blocks (each block may hold many
@@ -71,7 +93,7 @@ pub fn run(_root: &Path, exact: bool) -> Result<(), String> {
             continue;
         };
         let mut c = Counts::default();
-        walk(&dir, &mut c);
+        walk(&dir, &mut c)?;
         let ex = if exact { exact_tests(&name) } else { None };
         rows.push((name, c, ex));
     }
@@ -108,11 +130,40 @@ pub fn run(_root: &Path, exact: bool) -> Result<(), String> {
 
 #[cfg(test)]
 mod tests {
-    use super::count_tests;
+    use super::{Counts, count_tests, walk};
 
     #[test]
     fn counts_attributes_and_proptest_blocks() {
         let src = "#[test]\nfn a() {}\n// #[test] commented\n    #[test]\nfn b() {}\nproptest! {\n}\n";
         assert_eq!(count_tests(src), (2, 1));
     }
+
+    #[cfg(unix)]
+    #[test]
+    fn walk_does_not_follow_directory_symlinks() {
+        use std::os::unix::fs::symlink;
+        let base = std::env::temp_dir().join(format!("cadcraft-xtask-{}", std::process::id()));
+        let _ = std::fs::remove_dir_all(&base);
+        let real = base.join("real");
+        std::fs::create_dir_all(&real).unwrap();
+        std::fs::write(real.join("a.rs"), "fn a() {}\n").unwrap();
+        // A directory symlink pointing back at `real` would be re-scanned if followed.
+        symlink(&real, base.join("alias")).unwrap();
+        let mut c = Counts::default();
+        walk(&base, &mut c).unwrap();
+        assert_eq!(c.files, 1, "the aliased directory was followed and counted twice");
+        std::fs::remove_dir_all(&base).ok();
+    }
+    #[test]
+    fn walk_reports_budget_exhaustion() {
+        let base = std::env::temp_dir().join(format!("cadcraft-stats-budget-{}", std::process::id()));
+        std::fs::create_dir_all(&base).unwrap();
+        std::fs::write(base.join("a.rs"), "fn a() {}\n").unwrap();
+        let error = super::walk_inner(&base, &mut Counts::default(), &mut 0).unwrap_err();
+        assert!(error.contains("budget exhausted"));
+        let mut c = Counts::default();
+        super::walk_inner(&base, &mut c, &mut 1).unwrap();
+        assert_eq!(c.files, 1);
+        std::fs::remove_dir_all(base).unwrap();
+    }
 }