fix: handle TLS deallocation (#324)

Previously, creating a temporary file from a TLS destructor could panic
in fastrand (because the thread-local RNG may have been deallocated).

Now, we fork the RNG before we create each file, falling back on an RNG
with a static seed if the thread-local RNG has been deallocated.

Two downsides to this patch:

1. Temporary files created during TLS deallocation will have extremely
predictable names until the `getrandom` re-seed kicks in (assuming that
feature is enabled). IMO, that's fine; this would panic previously.
2. `getrandom` re-seeding used to re-randomize to the entire per-thread
RNG, now it only applies to the per-filename RNG. However, the will
still serve its purpose as a mitigation against potential DoS attacks.

I also considered managing the thread-local RNG myself instead of
relying on fastrand, but that just isn't worth the added code, IMO.

Thanks to @stoeckmann for reporting this and explaining the issue to me.
I went with this version instead of their version because I needed to
keep `tmpname` as a separate function for some tempfile v4 changes.

fixes #281

Author: Stebalien

src/util.rs

@@ -4,15 +4,15 @@ use std::{io, iter::repeat_with};
 
 use crate::error::IoResultExt;
 
-fn tmpname(prefix: &OsStr, suffix: &OsStr, rand_len: usize) -> OsString {
+fn tmpname(rng: &mut fastrand::Rng, prefix: &OsStr, suffix: &OsStr, rand_len: usize) -> OsString {
     let capacity = prefix
         .len()
         .saturating_add(suffix.len())
         .saturating_add(rand_len);
     let mut buf = OsString::with_capacity(capacity);
     buf.push(prefix);
     let mut char_buf = [0u8; 4];
-    for c in repeat_with(fastrand::alphanumeric).take(rand_len) {
+    for c in repeat_with(|| rng.alphanumeric()).take(rand_len) {
         buf.push(c.encode_utf8(&mut char_buf));
     }
     buf.push(suffix);
@@ -42,6 +42,8 @@ pub fn create_helper<R>(
         1
     };
 
+    // We fork the fastrand rng.
+    let mut rng = fastrand::Rng::new();
     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
@@ -58,10 +60,10 @@ pub fn create_helper<R>(
         if i == 3 {
             let mut seed = [0u8; 8];
             if getrandom::fill(&mut seed).is_ok() {
-                fastrand::seed(u64::from_ne_bytes(seed));
+                rng.seed(u64::from_ne_bytes(seed));
             }
         }
-        let path = base.join(tmpname(prefix, suffix, random_len));
+        let path = base.join(tmpname(&mut rng, prefix, suffix, random_len));
         return match f(path) {
             Err(ref e) if e.kind() == io::ErrorKind::AlreadyExists && num_retries > 1 => continue,
             // AddrInUse can happen if we're creating a UNIX domain socket and

tests/namedtempfile.rs

@@ -2,7 +2,7 @@
 
 use std::ffi::{OsStr, OsString};
 use std::fs::File;
-use std::io::{Read, Seek, SeekFrom, Write};
+use std::io::{self, Read, Seek, SeekFrom, Write};
 use std::path::{Path, PathBuf};
 use tempfile::{env, tempdir, Builder, NamedTempFile, TempPath};
 
@@ -469,17 +469,24 @@ fn test_reseed() {
     // Deterministic seed.
     fastrand::seed(42);
 
+    // I need to create 5 conflicts but I can't just make 5 temporary files because we fork the RNG
+    // each time we create a file.
     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();
+    let mut files: Vec<_> = Vec::new();
+    let _ = Builder::new().make(|path| -> io::Result<File> {
+        if attempts == 5 {
+            return Err(io::Error::new(io::ErrorKind::Other, "stop!"));
+        }
+        attempts += 1;
+        let f = File::options()
+            .write(true)
+            .create_new(true)
+            .open(path)
+            .unwrap();
+
+        files.push(NamedTempFile::from_parts(f, TempPath::from_path(path)));
+        Err(io::Error::new(io::ErrorKind::AlreadyExists, "fake!"))
+    });
 
     assert_eq!(5, attempts);
     attempts = 0;