feat: re-seed from system randomness on collision (#314)

Re-seed thread-local RNG from system randomness if we run into a
temporary file-name collision. This should address the concerns about
using a predictable RNG without hurting performance in the common case
where nobody is trying to predict our filenames. I'm only re-seeding
once because if we _still_ fail to create a temporary file, the
collision was likely due to too many temporary files instead of an
attacker predicting our random temporary file names.

I've also reduced the number of tries from 2^31 to 2^16. If it takes
more than that to create a temporary file, something else is wrong.
Pausing for a long time is usually worse than just failing.

fixes #178

Author: Stebalien

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## 3.15.0
+
+Re-seed the per-thread RNG from system randomness when we repeatedly fail to create temporary files (#314). This resolves a potential DoS vector (#178) while avoiding `getrandom` in the common case where it's necessary. The feature is optional but enabled by default via the `getrandom` feature.
+
+For libc-free builds, you'll either need to disable this feature or opt-in to a different [`getrandom` backend](https://github.com/rust-random/getrandom?tab=readme-ov-file#opt-in-backends).
+
 ## 3.14.0
 
 - Make the wasip2 target work (requires tempfile's "nightly" feature to be enabled). [#305](https://github.com/Stebalien/tempfile/pull/305).

Cargo.toml

@@ -22,6 +22,9 @@ fastrand = "2.1.1"
 # Not available in stdlib until 1.70, but we support 1.63 to support Debian stable.
 once_cell = { version = "1.19.0", default-features = false, features = ["std"] }
 
+[target.'cfg(any(unix, windows, target_os = "wasi"))'.dependencies]
+getrandom = { version = "0.2.15", default-features = false, optional = true }
+
 [target.'cfg(any(unix, target_os = "wasi"))'.dependencies]
 rustix = { version = "0.38.39", features = ["fs"] }
 
@@ -36,4 +39,5 @@ features = [
 doc-comment = "0.3"
 
 [features]
+default = ["getrandom"]
 nightly = []

src/lib.rs

@@ -29,7 +29,7 @@
 //! `tempfile` doesn't rely on file paths so this isn't an issue. However, `NamedTempFile` does
 //! rely on file paths for _some_ operations. See the security documentation on
 //! the `NamedTempFile` type for more information.
-//! 
+//!
 //! The OWASP Foundation provides a resource on vulnerabilities concerning insecure
 //! temporary files: https://owasp.org/www-community/vulnerabilities/Insecure_Temporary_File
 //!
@@ -150,7 +150,7 @@
 #[cfg(doctest)]
 doc_comment::doctest!("../README.md");
 
-const NUM_RETRIES: u32 = 1 << 31;
+const NUM_RETRIES: u32 = 65536;
 const NUM_RAND_CHARS: usize = 6;
 
 use std::ffi::OsStr;

src/util.rs

@@ -32,7 +32,25 @@ pub fn create_helper<R>(
         1
     };
 
-    for _ in 0..num_retries {
+    for i in 0..num_retries {
+        // If we fail to create the file the first three times, re-seed from system randomness in
+        // case an attacker is predicting our randomness (fastrand is predictable). If re-seeding
+        // doesn't help, either:
+        //
+        // 1. We have lots of temporary files, possibly created by an attacker but not necessarily.
+        //    Re-seeding the randomness won't help here.
+        // 2. We're failing to create random files for some other reason. This shouldn't be the case
+        //    given that we're checking error kinds, but it could happen.
+        #[cfg(all(
+            feature = "getrandom",
+            any(windows, unix, target_os = "redox", target_os = "wasi")
+        ))]
+        if i == 3 {
+            let mut seed = [0u8; 8];
+            if getrandom::getrandom(&mut seed).is_ok() {
+                fastrand::seed(u64::from_ne_bytes(seed));
+            }
+        }
         let path = base.join(tmpname(prefix, suffix, random_len));
         return match f(path) {
             Err(ref e) if e.kind() == io::ErrorKind::AlreadyExists && num_retries > 1 => continue,

tests/namedtempfile.rs

@@ -421,55 +421,73 @@ fn test_make_uds() {
 #[cfg(unix)]
 #[test]
 fn test_make_uds_conflict() {
+    use std::io::ErrorKind;
     use std::os::unix::net::UnixListener;
-    use std::sync::atomic::{AtomicUsize, Ordering};
-    use std::sync::Arc;
-
-    // Check that retries happen correctly by racing N different threads.
-
-    const NTHREADS: usize = 20;
-
-    // The number of times our callback was called.
-    let tries = Arc::new(AtomicUsize::new(0));
-
-    let mut threads = Vec::with_capacity(NTHREADS);
-
-    for _ in 0..NTHREADS {
-        let tries = tries.clone();
-        threads.push(std::thread::spawn(move || {
-            // Ensure that every thread uses the same seed so we are guaranteed
-            // to retry. Note that fastrand seeds are thread-local.
-            fastrand::seed(42);
-
-            Builder::new()
-                .prefix("tmp")
-                .suffix(".sock")
-                .rand_bytes(12)
-                .make(|path| {
-                    tries.fetch_add(1, Ordering::Relaxed);
-                    UnixListener::bind(path)
-                })
-        }));
-    }
 
-    // Join all threads, but don't drop the temp file yet. Otherwise, we won't
-    // get a deterministic number of `tries`.
-    let sockets: Vec<_> = threads
-        .into_iter()
-        .map(|thread| thread.join().unwrap().unwrap())
-        .collect();
-
-    // Number of tries is exactly equal to (n*(n+1))/2.
-    assert_eq!(
-        tries.load(Ordering::Relaxed),
-        (NTHREADS * (NTHREADS + 1)) / 2
-    );
+    let sockets = std::iter::repeat_with(|| {
+        Builder::new()
+            .prefix("tmp")
+            .suffix(".sock")
+            .rand_bytes(1)
+            .make(|path| UnixListener::bind(path))
+    })
+    .take_while(|r| match r {
+        Ok(_) => true,
+        Err(e) if matches!(e.kind(), ErrorKind::AddrInUse | ErrorKind::AlreadyExists) => false,
+        Err(e) => panic!("unexpected error {e}"),
+    })
+    .collect::<Result<Vec<_>, _>>()
+    .unwrap();
+
+    // Number of sockets we can create. Depends on whether or not the filesystem is case sensitive.
+
+    #[cfg(target_os = "macos")]
+    const NUM_FILES: usize = 36;
+    #[cfg(not(target_os = "macos"))]
+    const NUM_FILES: usize = 62;
+
+    assert_eq!(sockets.len(), NUM_FILES);
 
     for socket in sockets {
         assert!(socket.path().exists());
     }
 }
 
+/// Make sure we re-seed with system randomness if we run into a conflict.
+#[test]
+fn test_reseed() {
+    // Deterministic seed.
+    fastrand::seed(42);
+
+    let mut attempts = 0;
+    let _files: Vec<_> = std::iter::repeat_with(|| {
+        Builder::new()
+            .make(|path| {
+                attempts += 1;
+                File::options().write(true).create_new(true).open(path)
+            })
+            .unwrap()
+    })
+    .take(5)
+    .collect();
+
+    assert_eq!(5, attempts);
+    attempts = 0;
+
+    // Re-seed to cause a conflict.
+    fastrand::seed(42);
+
+    let _f = Builder::new()
+        .make(|path| {
+            attempts += 1;
+            File::options().write(true).create_new(true).open(path)
+        })
+        .unwrap();
+
+    // We expect exactly three conflict before we re-seed with system randomness.
+    assert_eq!(4, attempts);
+}
+
 // Issue #224.
 #[test]
 fn test_overly_generic_bounds() {