Prevent mutating the global environment pointer in CommandExt::exec and opt to use execve and resolve path manually

library/std/src/sys/pal/unix/conf.rs

@@ -6,13 +6,11 @@ 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).
+/// Returns the value for [`confstr(key, ...)`][posix_confstr].
 ///
 /// [posix_confstr]:
 ///     https://pubs.opengroup.org/onlinepubs/9799919799/functions/confstr.html
-#[cfg(target_vendor = "apple")]
+#[cfg(any(target_vendor = "apple", all(target_os = "linux", target_env = "gnu")))]
 pub fn confstr(
     key: crate::ffi::c_int,
     size_hint: Option<usize>,

library/std/src/sys/process/unix/common.rs

@@ -91,6 +91,7 @@ pub struct Command {
     program: CString,
     args: CStringArray,
     env: CommandEnv,
+    default_path: OsString,
 
     program_kind: ProgramKind,
     cwd: Option<CString>,
@@ -163,15 +164,51 @@ impl ProgramKind {
 
 impl Command {
     pub fn new(program: &OsStr) -> Command {
+        #[cfg(any(
+            all(target_os = "linux", not(target_env = "gnu")),
+            target_vendor = "apple",
+            target_os = "android",
+            target_os = "openbsd",
+            target_os = "freebsd",
+            target_os = "dragonfly"
+        ))]
+        use crate::os::unix::ffi::OsStrExt;
+        #[cfg(all(target_os = "linux", target_env = "gnu"))]
+        use crate::sys::conf::confstr;
+
         let mut saw_nul = false;
         let program_kind = ProgramKind::new(program.as_ref());
         let program = os2c(program, &mut saw_nul);
         let mut args = CStringArray::with_capacity(1);
         args.push(program.clone());
+
         Command {
             program,
             args,
             env: Default::default(),
+            default_path: cfg_select! {
+                all(target_os = "linux", target_env = "gnu") => {
+                    confstr(libc::_CS_PATH, None).unwrap_or(OsString::new())
+                }
+                any(
+                    all(target_os = "linux", not(target_env = "gnu")),
+                    target_vendor = "apple",
+                    target_os = "android",
+                    target_os = "openbsd",
+                    target_os = "freebsd",
+                    target_os = "dragonfly",
+                ) => {
+                    // SAFETY: libc::_PATH_DEFPATH is a nul-terminated char pointer
+                    unsafe {
+                        OsStr::from_bytes(crate::slice::from_raw_parts(
+                            libc::_PATH_DEFPATH.cast(),
+                            libc::strlen(libc::_PATH_DEFPATH),
+                        ))
+                        .into()
+                    }
+                }
+                _ => OsString::new(),
+            },
             program_kind,
             cwd: None,
             chroot: None,
@@ -271,6 +308,10 @@ impl Command {
         self.env.does_clear()
     }
 
+    pub fn get_default_path(&self) -> &OsStr {
+        &self.default_path
+    }
+
     pub fn get_resolved_envs(&self) -> CommandResolvedEnvs {
         CommandResolvedEnvs::new(self.env.capture())
     }

library/std/src/sys/process/unix/common/cstring_array.rs

@@ -32,6 +32,11 @@ impl CStringArray {
         drop(unsafe { CString::from_raw(old.cast_mut()) });
     }
 
+    /// Returns the length of the array (null pointer excluded).
+    pub fn len(&self) -> usize {
+        self.ptrs.len() - 1
+    }
+
     /// Push an additional string to the array.
     pub fn push(&mut self, item: CString) {
         let argc = self.ptrs.len() - 1;

library/std/src/sys/process/unix/unix.rs

@@ -91,6 +91,33 @@ impl Command {
         // The child calls `mem::forget` to leak the lock, which is crucial because
         // releasing a lock is not async-signal-safe.
         let env_lock = sys::env::env_read_lock();
+        let paths = self.find_path_var(envp.as_ref());
+        let max_path_len = self.compute_max_len(paths);
+        let mut buf = Box::new_uninit_slice(max_path_len);
+        let args = self.get_argv();
+        // Contains the shell path, the file to exec, the argument list, and nul ptr
+        let mut shell_argv = Vec::with_capacity(2 + args.len());
+
+        let call_shell = if cfg!(any(
+            target_vendor = "apple",
+            target_os = "android",
+            target_os = "openbsd",
+            target_os = "freebsd",
+            target_os = "dragonfly",
+            all(target_os = "linux", not(target_env = "musl")),
+            target_os = "haiku"
+        )) {
+            shell_argv.push(libc::_PATH_BSHELL);
+            // This value is updated in do_exec with the actual file to exec on.
+            shell_argv.push(crate::ptr::null());
+            // argv[0] contains the program name, but we are only interested in the
+            // arguments so skip it.
+            args.iter().skip(1).for_each(|arg| shell_argv.push(arg.as_ptr()));
+            true
+        } else {
+            false
+        };
+
         let pid = unsafe { self.do_fork()? };
 
         if pid == 0 {
@@ -101,7 +128,9 @@ impl Command {
             if self.get_create_pidfd() {
                 self.send_pidfd(&output);
             }
-            let Err(err) = unsafe { self.do_exec(theirs, envp.as_ref()) };
+            let Err(err) = unsafe {
+                self.do_exec(theirs, envp.as_ref(), paths, &mut buf, &mut shell_argv, call_shell)
+            };
             let errno = err.raw_os_error().unwrap_or(libc::EINVAL) as u32;
             let errno = errno.to_be_bytes();
             let bytes = [
@@ -240,15 +269,206 @@ impl Command {
                     // the environment is synchronized, so make sure to grab the
                     // environment lock before we try to exec.
                     let _lock = sys::env::env_read_lock();
-
-                    let Err(e) = self.do_exec(theirs, envp.as_ref());
+                    let paths = self.find_path_var(envp.as_ref());
+                    let max_path_len = self.compute_max_len(paths);
+                    let mut buf = Box::new_uninit_slice(max_path_len);
+                    let args = self.get_argv();
+                    let mut shell_argv = Vec::with_capacity(2 + args.len());
+
+                    let call_shell = if cfg!(any(
+                        target_vendor = "apple",
+                        target_os = "android",
+                        target_os = "openbsd",
+                        target_os = "netbsd",
+                        target_os = "freebsd",
+                        target_os = "dragonfly",
+                        all(target_os = "linux", not(target_env = "musl")),
+                        target_os = "haiku"
+                    )) {
+                        shell_argv.push(libc::_PATH_BSHELL);
+                        // This value is updated in do_exec with the actual file to exec on.
+                        shell_argv.push(crate::ptr::null());
+                        // argv[0] contains the program name, but we are only interested in the
+                        // arguments so skip it.
+                        args.iter().skip(1).for_each(|arg| shell_argv.push(arg.as_ptr()));
+                        true
+                    } else {
+                        false
+                    };
+                    let Err(e) = self.do_exec(
+                        theirs,
+                        envp.as_ref(),
+                        paths,
+                        &mut buf,
+                        &mut shell_argv,
+                        call_shell,
+                    );
                     e
                 }
             }
             Err(e) => e,
         }
     }
 
+    /// Helper function to locate the PATH environment variable from an environ pointer.
+    /// This omits the "PATH=" value and returns just the colon delimited paths. The lifetime
+    /// of the resulting u8 slice is dependent on our passed in environment pointer and not
+    /// `Command`, so we don't have a lasting immutable borrow occurring on `Command`, which
+    /// will conflict with mutable borrow that will occur later on `Command` through
+    /// `Command::do_exec`.
+    ///
+    /// If we could not locate the PATH environment variable, this function will return `None`.
+    fn find_path_var<'a>(&self, maybe_envp: Option<&'a CStringArray>) -> Option<&'a [u8]> {
+        let mut envp_ptr =
+            maybe_envp.map(|envp| envp.as_ptr()).unwrap_or(unsafe { *sys::env::environ() });
+
+        if !envp_ptr.is_null() {
+            // SAFETY: We should be guaranteed to have a nul terminated array either via
+            // from CStringArray or from what `environ()` returns
+            unsafe {
+                while !(*envp_ptr).is_null() {
+                    let c_str_as_bytes = crate::ffi::CStr::from_ptr(*envp_ptr).to_bytes();
+                    if c_str_as_bytes.starts_with(b"PATH=") {
+                        return Some(&c_str_as_bytes[5..]);
+                    } else {
+                        envp_ptr = envp_ptr.add(1);
+                    }
+                }
+            }
+            None
+        } else {
+            None
+        }
+    }
+
+    /// Helper function to calculate the maximum path len we'll see from the concatenation
+    /// of a PATH environment + the program cstr + a separator byte. The returned value
+    /// is used to pre-allocate a buffer that will be used for the entire duration of
+    /// `Command::do_exec()`.
+    fn compute_max_len(&self, paths: Option<&[u8]>) -> usize {
+        // Defer to default path from `_CS_PATH_` if PATH environment variable was
+        // not found
+        let paths = paths.unwrap_or(self.get_default_path().as_encoded_bytes());
+        let longest_path_len = paths.split(|b| *b == b':').map(|path| path.len()).max();
+
+        // Parsed path len from envp + file path len + 1 for separator byte + 1 for nul terminated byte
+        if let Some(longest_path_len) = longest_path_len {
+            return longest_path_len + self.get_program_cstr().count_bytes() + 2;
+        }
+
+        // Empty paths do not need a separator byte, so this pre-allocates only for the file
+        // and nul byte
+        self.get_program_cstr().count_bytes() + 1
+    }
+
+    /// Retrieves the last error set by `libc::execve` from `Command::execvpe`.
+    unsafe fn last_execvpe_errno(
+        &self,
+        file: *const libc::c_char,
+        shell_argv: &mut Vec<*const libc::c_char>,
+        envp_ptr: *const *const libc::c_char,
+        got_perm_denied: &mut bool,
+        call_shell: bool,
+    ) -> Result<(), io::Error> {
+        let err = crate::sys::io::errno();
+
+        // If execve returns `ENOEXEC`, the file is accessible but it is not an
+        // executable file; invoke shell to interpret as a script
+        if err == libc::ENOEXEC && call_shell {
+            shell_argv[1] = file;
+            libc::execve(shell_argv[0], shell_argv.as_ptr(), envp_ptr);
+        }
+
+        match err {
+            // Record that we got a 'Permission Denied' error in the event that if we
+            // find no executable to use, we should report that there was a usable
+            // executable but we were denied access to it.
+            libc::EACCES => *got_perm_denied = true,
+            // Try the next path.
+            libc::ENOENT | libc::ESTALE | libc::ENOTDIR | libc::ENODEV | libc::ETIMEDOUT => {}
+            // Any other error returned by execve should be returned. For example,
+            // ENAMETOOLONG is an error that is returned due to security risks on
+            // executing the wrong program through setting a very long path
+            _ => return Err(io::Error::from_raw_os_error(err)),
+        }
+
+        Ok(())
+    }
+
+    /// An internal helper function that replicates the behavior of execvpe for all unix
+    /// platforms. Not all unix platforms support `execvpe` syscall, so this needs to be
+    /// implemented manually through `execve`. Thix exec function is called in the context
+    /// of the current process; all forking must be done outside of this function (e.g.
+    /// `Command::exec` does not fork, `Command::spawn` does fork).
+    unsafe fn execvpe(
+        &self,
+        maybe_envp: Option<&CStringArray>,
+        paths: Option<&[u8]>,
+        buf: &mut [crate::mem::MaybeUninit<u8>],
+        shell_argv: &mut Vec<*const libc::c_char>,
+        call_shell: bool,
+    ) -> Result<!, io::Error> {
+        let file = self.get_program_cstr();
+        let file_as_bytes = file.to_bytes_with_nul();
+        let argv = self.get_argv();
+        let envp_ptr =
+            maybe_envp.map(|envp| envp.as_ptr()).unwrap_or(unsafe { *sys::env::environ() });
+        let paths = paths.unwrap_or(self.get_default_path().as_encoded_bytes());
+        let mut got_perm_denied = false;
+
+        // Path searching does not occur when our file contains a `/`
+        if file_as_bytes.contains(&b'/') {
+            libc::execve(file.as_ptr(), argv.as_ptr(), envp_ptr);
+            self.last_execvpe_errno(
+                file.as_ptr(),
+                shell_argv,
+                envp_ptr,
+                &mut got_perm_denied,
+                call_shell,
+            )?;
+        } else {
+            for path in paths.split(|b| *b == b':') {
+                let path_len = path.len();
+
+                // Copy path to execute in the pre-allocated buffer accordingly
+                // Use the current path entry, plus a '/' if nonempty, plus the file to
+                // execute.
+                if path_len != 0 {
+                    buf[0..path_len].write_copy_of_slice(path);
+                    if path[path_len - 1] != b'/' {
+                        buf[path_len].write(b'/');
+                        buf[path_len + 1..path_len + 1 + file_as_bytes.len()]
+                            .write_copy_of_slice(file_as_bytes);
+                    } else {
+                        buf[path_len..path_len + file_as_bytes.len()]
+                            .write_copy_of_slice(file_as_bytes);
+                    }
+                } else {
+                    buf[0..file_as_bytes.len()].write_copy_of_slice(file_as_bytes);
+                }
+
+                // Try executing on path
+                libc::execve(buf.as_ptr().cast(), argv.as_ptr(), envp_ptr);
+                self.last_execvpe_errno(
+                    buf.as_ptr().cast(),
+                    shell_argv,
+                    envp_ptr,
+                    &mut got_perm_denied,
+                    call_shell,
+                )?;
+            }
+        }
+
+        // At least one failure was due to lack of permissions, so we should
+        // report that failure first
+        if got_perm_denied {
+            return Err(io::Error::from_raw_os_error(libc::EACCES));
+        }
+
+        // No paths were executable
+        Err(io::Error::from_raw_os_error(libc::ENOENT))
+    }
+
     // And at this point we've reached a special time in the life of the
     // child. The child must now be considered hamstrung and unable to
     // do anything other than syscalls really. Consider the following
@@ -284,6 +504,10 @@ impl Command {
         &mut self,
         stdio: ChildPipes,
         maybe_envp: Option<&CStringArray>,
+        paths: Option<&[u8]>,
+        buf: &mut [crate::mem::MaybeUninit<u8>],
+        shell_argv: &mut Vec<*const libc::c_char>,
+        call_shell: bool,
     ) -> Result<!, io::Error> {
         use crate::sys::{self, cvt_r};
 
@@ -389,29 +613,7 @@ impl Command {
             callback()?;
         }
 
-        // Although we're performing an exec here we may also return with an
-        // error from this function (without actually exec'ing) in which case we
-        // want to be sure to restore the global environment back to what it
-        // once was, ensuring that our temporary override, when free'd, doesn't
-        // corrupt our process's environment.
-        let mut _reset = None;
-        if let Some(envp) = maybe_envp {
-            struct Reset(*const *const libc::c_char);
-
-            impl Drop for Reset {
-                fn drop(&mut self) {
-                    unsafe {
-                        *sys::env::environ() = self.0;
-                    }
-                }
-            }
-
-            _reset = Some(Reset(*sys::env::environ()));
-            *sys::env::environ() = envp.as_ptr();
-        }
-
-        libc::execvp(self.get_program_cstr().as_ptr(), self.get_argv().as_ptr());
-        Err(io::Error::last_os_error())
+        self.execvpe(maybe_envp, paths, buf, shell_argv, call_shell)
     }
 
     #[cfg(any(target_os = "tvos", target_os = "watchos"))]