rollup merge of rust-lang#20488: ltratt/nondeterministic_tempdir

The previous scheme made it possible for another user/attacker to cause the
temporary directory creation scheme to panic. All you needed to know was the pid
of the process you wanted to target ('other_pid') and the suffix it was using
(let's pretend it's 'sfx') and then code such as this would, in essence, DOS it:

    for i in range(0u, 1001) {
        let tp = &Path::new(format!("/tmp/rs-{}-{}-sfx", other_pid, i));
        match fs::mkdir(tp, io::USER_RWX) { _ => () }
    }

Since the scheme only 1000 times to create a temporary directory before dying,
the next time the attacked process called TempDir::new("sfx") after that would
typically cause a panic. Of course, you don't necessarily need an attacker to
cause such a DOS: creating 1000 temporary directories without closing any of the
previous would be enough to DOS yourself.

This patch broadly follows the OpenBSD implementation of mkstemp. It uses the
operating system's random number generator to produce random directory names
that are impractical to guess (and, just in case someone manages to do that, it
retries creating the directory for a long time before giving up; OpenBSD
retries INT_MAX times, although 1<<31 seems enough to thwart even the most
patient attacker).

As a small additional change while the file name is changing, this patch also
makes the argument that TempDir::new takes a prefix rather than a suffix.
This is because 1) it more closely matches what mkstemp and friends do 2)
if you're going to have a deterministic part of a filename, you really want it at
the beginning so that shell completion is useful.

Author: alexcrichton

src/libstd/io/tempfile.rs

@@ -10,16 +10,18 @@
 
 //! Temporary files and directories
 
-use io::{fs, IoResult};
+use io::{fs, IoError, IoErrorKind, IoResult};
 use io;
-use libc;
+use iter::{IteratorExt, range};
 use ops::Drop;
 use option::Option;
 use option::Option::{None, Some};
 use os;
 use path::{Path, GenericPath};
+use rand::{Rng, thread_rng};
 use result::Result::{Ok, Err};
-use sync::atomic::{AtomicUint, ATOMIC_UINT_INIT, Ordering};
+use str::StrExt;
+use string::String;
 
 /// A wrapper for a path to temporary directory implementing automatic
 /// scope-based deletion.
@@ -31,7 +33,7 @@ use sync::atomic::{AtomicUint, ATOMIC_UINT_INIT, Ordering};
 ///
 /// {
 ///     // create a temporary directory
-///     let tmpdir = match TempDir::new("mysuffix") {
+///     let tmpdir = match TempDir::new("myprefix") {
 ///         Ok(dir) => dir,
 ///         Err(e) => panic!("couldn't create temporary directory: {}", e)
 ///     };
@@ -46,7 +48,7 @@ use sync::atomic::{AtomicUint, ATOMIC_UINT_INIT, Ordering};
 /// }
 /// {
 ///     // create a temporary directory, this time using a custom path
-///     let tmpdir = match TempDir::new_in(&Path::new("/tmp/best/custom/path"), "mysuffix") {
+///     let tmpdir = match TempDir::new_in(&Path::new("/tmp/best/custom/path"), "myprefix") {
 ///         Ok(dir) => dir,
 ///         Err(e) => panic!("couldn't create temporary directory: {}", e)
 ///     };
@@ -61,7 +63,7 @@ use sync::atomic::{AtomicUint, ATOMIC_UINT_INIT, Ordering};
 /// }
 /// {
 ///     // create a temporary directory
-///     let tmpdir = match TempDir::new("mysuffix") {
+///     let tmpdir = match TempDir::new("myprefix") {
 ///         Ok(dir) => dir,
 ///         Err(e) => panic!("couldn't create temporary directory: {}", e)
 ///     };
@@ -78,47 +80,59 @@ pub struct TempDir {
     disarmed: bool
 }
 
+// How many times should we (re)try finding an unused random name? It should be
+// enough that an attacker will run out of luck before we run out of patience.
+const NUM_RETRIES: u32 = 1 << 31;
+// How many characters should we include in a random file name? It needs to
+// be enough to dissuade an attacker from trying to preemptively create names
+// of that length, but not so huge that we unnecessarily drain the random number
+// generator of entropy.
+const NUM_RAND_CHARS: uint = 12;
+
 impl TempDir {
     /// Attempts to make a temporary directory inside of `tmpdir` whose name
-    /// will have the suffix `suffix`. The directory will be automatically
+    /// will have the prefix `prefix`. The directory will be automatically
     /// deleted once the returned wrapper is destroyed.
     ///
     /// If no directory can be created, `Err` is returned.
-    pub fn new_in(tmpdir: &Path, suffix: &str) -> IoResult<TempDir> {
+    pub fn new_in(tmpdir: &Path, prefix: &str) -> IoResult<TempDir> {
         if !tmpdir.is_absolute() {
             let abs_tmpdir = try!(os::make_absolute(tmpdir));
-            return TempDir::new_in(&abs_tmpdir, suffix);
+            return TempDir::new_in(&abs_tmpdir, prefix);
         }
 
-        static CNT: AtomicUint = ATOMIC_UINT_INIT;
-
-        let mut attempts = 0u;
-        loop {
-            let filename =
-                format!("rs-{}-{}-{}",
-                        unsafe { libc::getpid() },
-                        CNT.fetch_add(1, Ordering::SeqCst),
-                        suffix);
-            let p = tmpdir.join(filename);
-            match fs::mkdir(&p, io::USER_RWX) {
-                Err(error) => {
-                    if attempts >= 1000 {
-                        return Err(error)
-                    }
-                    attempts += 1;
-                }
-                Ok(()) => return Ok(TempDir { path: Some(p), disarmed: false })
+        let mut rng = thread_rng();
+        for _ in range(0, NUM_RETRIES) {
+            let suffix: String = rng.gen_ascii_chars().take(NUM_RAND_CHARS).collect();
+            let leaf = if prefix.len() > 0 {
+                format!("{}.{}", prefix, suffix)
+            } else {
+                // If we're given an empty string for a prefix, then creating a
+                // directory starting with "." would lead to it being
+                // semi-invisible on some systems.
+                suffix
+            };
+            let path = tmpdir.join(leaf);
+            match fs::mkdir(&path, io::USER_RWX) {
+                Ok(_) => return Ok(TempDir { path: Some(path), disarmed: false }),
+                Err(IoError{kind:IoErrorKind::PathAlreadyExists,..}) => (),
+                Err(e) => return Err(e)
             }
         }
+
+        return Err(IoError{
+                       kind: IoErrorKind::PathAlreadyExists,
+                       desc:"Exhausted",
+                       detail: None});
     }
 
     /// Attempts to make a temporary directory inside of `os::tmpdir()` whose
-    /// name will have the suffix `suffix`. The directory will be automatically
+    /// name will have the prefix `prefix`. The directory will be automatically
     /// deleted once the returned wrapper is destroyed.
     ///
     /// If no directory can be created, `Err` is returned.
-    pub fn new(suffix: &str) -> IoResult<TempDir> {
-        TempDir::new_in(&os::tmpdir(), suffix)
+    pub fn new(prefix: &str) -> IoResult<TempDir> {
+        TempDir::new_in(&os::tmpdir(), prefix)
     }
 
     /// Unwrap the wrapped `std::path::Path` from the `TempDir` wrapper.

src/test/run-pass/tempfile.rs

@@ -29,7 +29,7 @@ fn test_tempdir() {
     let path = {
         let p = TempDir::new_in(&Path::new("."), "foobar").unwrap();
         let p = p.path();
-        assert!(p.as_vec().ends_with(b"foobar"));
+        assert!(p.as_str().unwrap().contains("foobar"));
         p.clone()
     };
     assert!(!path.exists());