diff --git a/library/std/src/env.rs b/library/std/src/env.rs index d732a15117e9e..27f4daba44bf6 100644 --- a/library/std/src/env.rs +++ b/library/std/src/env.rs @@ -653,19 +653,28 @@ pub fn home_dir() -> Option { /// may result in "insecure temporary file" security vulnerabilities. Consider /// using a crate that securely creates temporary files or directories. /// +/// Note that the returned value may be a symbolic link, not a directory. +/// /// # Platform-specific behavior /// /// On Unix, returns the value of the `TMPDIR` environment variable if it is -/// set, otherwise for non-Android it returns `/tmp`. On Android, since there -/// is no global temporary folder (it is usually allocated per-app), it returns -/// `/data/local/tmp`. +/// set, otherwise the value is OS-specific: +/// - On Android, there is no global temporary folder (it is usually allocated +/// per-app), it returns `/data/local/tmp`. +/// - On Darwin-based OSes (macOS, iOS, etc) it returns the directory provided +/// by `confstr(_CS_DARWIN_USER_TEMP_DIR, ...)`, as recommended by [Apple's +/// security guidelines][appledoc]. +/// - On all other unix-based OSes, it returns `/tmp`. +/// /// On Windows, the behavior is equivalent to that of [`GetTempPath2`][GetTempPath2] / /// [`GetTempPath`][GetTempPath], which this function uses internally. +/// /// Note that, this [may change in the future][changes]. /// /// [changes]: io#platform-specific-behavior /// [GetTempPath2]: https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-gettemppath2a /// [GetTempPath]: https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-gettemppatha +/// [appledoc]: https://developer.apple.com/library/archive/documentation/Security/Conceptual/SecureCodingGuide/Articles/RaceConditions.html#//apple_ref/doc/uid/TP40002585-SW10 /// /// ```no_run /// use std::env; diff --git a/library/std/src/sys/pal/unix/os.rs b/library/std/src/sys/pal/unix/os.rs index f983d174ed61c..f207131ddf332 100644 --- a/library/std/src/sys/pal/unix/os.rs +++ b/library/std/src/sys/pal/unix/os.rs @@ -698,12 +698,82 @@ pub fn page_size() -> usize { unsafe { libc::sysconf(libc::_SC_PAGESIZE) as usize } } +// Returns the value for [`confstr(key, ...)`][posix_confstr]. Currently only +// used on Darwin, but should work on any unix (in case we need to get +// `_CS_PATH` or `_CS_V[67]_ENV` in the future). +// +// [posix_confstr]: +// https://pubs.opengroup.org/onlinepubs/9699919799/functions/confstr.html +// +// FIXME: Support `confstr` in Miri. +#[cfg(all(target_vendor = "apple", not(miri)))] +fn confstr(key: c_int, size_hint: Option) -> io::Result { + let mut buf: Vec = Vec::with_capacity(0); + let mut bytes_needed_including_nul = size_hint + .unwrap_or_else(|| { + // Treat "None" as "do an extra call to get the length". In theory + // we could move this into the loop below, but it's hard to do given + // that it isn't 100% clear if it's legal to pass 0 for `len` when + // the buffer isn't null. + unsafe { libc::confstr(key, core::ptr::null_mut(), 0) } + }) + .max(1); + // If the value returned by `confstr` is greater than the len passed into + // it, then the value was truncated, meaning we need to retry. Note that + // while `confstr` results don't seem to change for a process, it's unclear + // if this is guaranteed anywhere, so looping does seem required. + while bytes_needed_including_nul > buf.capacity() { + // We write into the spare capacity of `buf`. This lets us avoid + // changing buf's `len`, which both simplifies `reserve` computation, + // allows working with `Vec` instead of `Vec>`, and + // may avoid a copy, since the Vec knows that none of the bytes are needed + // when reallocating (well, in theory anyway). + buf.reserve(bytes_needed_including_nul); + // `confstr` returns + // - 0 in the case of errors: we break and return an error. + // - The number of bytes written, iff the provided buffer is enough to + // hold the entire value: we break and return the data in `buf`. + // - Otherwise, the number of bytes needed (including nul): we go + // through the loop again. + bytes_needed_including_nul = + unsafe { libc::confstr(key, buf.as_mut_ptr().cast::(), buf.capacity()) }; + } + // `confstr` returns 0 in the case of an error. + if bytes_needed_including_nul == 0 { + return Err(io::Error::last_os_error()); + } + // Safety: `confstr(..., buf.as_mut_ptr(), buf.capacity())` returned a + // non-zero value, meaning `bytes_needed_including_nul` bytes were + // initialized. + unsafe { + buf.set_len(bytes_needed_including_nul); + // Remove the NUL-terminator. + let last_byte = buf.pop(); + // ... and smoke-check that it *was* a NUL-terminator. + assert_eq!(last_byte, Some(0), "`confstr` provided a string which wasn't nul-terminated"); + }; + Ok(OsString::from_vec(buf)) +} + +#[cfg(all(target_vendor = "apple", not(miri)))] +fn darwin_temp_dir() -> PathBuf { + confstr(libc::_CS_DARWIN_USER_TEMP_DIR, Some(64)).map(PathBuf::from).unwrap_or_else(|_| { + // It failed for whatever reason (there are several possible reasons), + // so return the global one. + PathBuf::from("/tmp") + }) +} + pub fn temp_dir() -> PathBuf { crate::env::var_os("TMPDIR").map(PathBuf::from).unwrap_or_else(|| { - if cfg!(target_os = "android") { - PathBuf::from("/data/local/tmp") - } else { - PathBuf::from("/tmp") + cfg_if::cfg_if! { + if #[cfg(all(target_vendor = "apple", not(miri)))] { + darwin_temp_dir() + } else if #[cfg(target_os = "android")] { + PathBuf::from("/data/local/tmp") + } else { + PathBuf::from("/tmp") + } } }) } diff --git a/library/std/src/sys/pal/unix/os/tests.rs b/library/std/src/sys/pal/unix/os/tests.rs index efc29955b05fe..63a1cc1e94a1d 100644 --- a/library/std/src/sys/pal/unix/os/tests.rs +++ b/library/std/src/sys/pal/unix/os/tests.rs @@ -21,3 +21,28 @@ fn test_parse_glibc_version() { assert_eq!(parsed, super::parse_glibc_version(version_str)); } } + +// Smoke check `confstr`, do it for several hint values, to ensure our resizing +// logic is correct. +#[test] +#[cfg(all(target_vendor = "apple", not(miri)))] +fn test_confstr() { + for key in [libc::_CS_DARWIN_USER_TEMP_DIR, libc::_CS_PATH] { + let value_nohint = super::confstr(key, None).unwrap_or_else(|e| { + panic!("confstr({key}, None) failed: {e:?}"); + }); + let end = (value_nohint.len() + 1) * 2; + for hint in 0..end { + assert_eq!( + super::confstr(key, Some(hint)).as_deref().ok(), + Some(&*value_nohint), + "confstr({key}, Some({hint})) failed", + ); + } + } + // Smoke check that we don't loop forever or something if the input was not valid. + for hint in [None, Some(0), Some(1)] { + let hopefully_invalid = 123456789_i32; + assert!(super::confstr(hopefully_invalid, hint).is_err()); + } +}