Auto merge of #55359 - alex:command-exec-uaf, r=alexcrichton
Fixes #46775 -- don't mutate the process's environment in Command::exec Instead, pass the environment to execvpe, so the kernel can apply it directly to the new process. This avoids a use-after-free in the case where exec'ing the new process fails for any reason, as well as a race condition if there are other threads alive during the exec. Fixes #46775
This commit is contained in:
commit
ad4c885225
|
@ -141,6 +141,10 @@ impl Command {
|
||||||
pub fn get_argv(&self) -> &Vec<*const c_char> {
|
pub fn get_argv(&self) -> &Vec<*const c_char> {
|
||||||
&self.argv.0
|
&self.argv.0
|
||||||
}
|
}
|
||||||
|
#[cfg(not(target_os = "fuchsia"))]
|
||||||
|
pub fn get_program(&self) -> &CString {
|
||||||
|
return &self.program;
|
||||||
|
}
|
||||||
|
|
||||||
#[allow(dead_code)]
|
#[allow(dead_code)]
|
||||||
pub fn get_cwd(&self) -> &Option<CString> {
|
pub fn get_cwd(&self) -> &Option<CString> {
|
||||||
|
@ -244,6 +248,10 @@ impl CStringArray {
|
||||||
pub fn as_ptr(&self) -> *const *const c_char {
|
pub fn as_ptr(&self) -> *const *const c_char {
|
||||||
self.ptrs.as_ptr()
|
self.ptrs.as_ptr()
|
||||||
}
|
}
|
||||||
|
#[cfg(not(target_os = "fuchsia"))]
|
||||||
|
pub fn get_items(&self) -> &[CString] {
|
||||||
|
return &self.items;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn construct_envp(env: BTreeMap<DefaultEnvKey, OsString>, saw_nul: &mut bool) -> CStringArray {
|
fn construct_envp(env: BTreeMap<DefaultEnvKey, OsString>, saw_nul: &mut bool) -> CStringArray {
|
||||||
|
|
|
@ -8,6 +8,8 @@
|
||||||
// option. This file may not be copied, modified, or distributed
|
// option. This file may not be copied, modified, or distributed
|
||||||
// except according to those terms.
|
// except according to those terms.
|
||||||
|
|
||||||
|
use env;
|
||||||
|
use ffi::CString;
|
||||||
use io::{self, Error, ErrorKind};
|
use io::{self, Error, ErrorKind};
|
||||||
use libc::{self, c_int, gid_t, pid_t, uid_t};
|
use libc::{self, c_int, gid_t, pid_t, uid_t};
|
||||||
use ptr;
|
use ptr;
|
||||||
|
@ -39,13 +41,15 @@ impl Command {
|
||||||
return Ok((ret, ours))
|
return Ok((ret, ours))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let possible_paths = self.compute_possible_paths(envp.as_ref());
|
||||||
|
|
||||||
let (input, output) = sys::pipe::anon_pipe()?;
|
let (input, output) = sys::pipe::anon_pipe()?;
|
||||||
|
|
||||||
let pid = unsafe {
|
let pid = unsafe {
|
||||||
match cvt(libc::fork())? {
|
match cvt(libc::fork())? {
|
||||||
0 => {
|
0 => {
|
||||||
drop(input);
|
drop(input);
|
||||||
let err = self.do_exec(theirs, envp.as_ref());
|
let err = self.do_exec(theirs, envp.as_ref(), possible_paths);
|
||||||
let errno = err.raw_os_error().unwrap_or(libc::EINVAL) as u32;
|
let errno = err.raw_os_error().unwrap_or(libc::EINVAL) as u32;
|
||||||
let bytes = [
|
let bytes = [
|
||||||
(errno >> 24) as u8,
|
(errno >> 24) as u8,
|
||||||
|
@ -113,12 +117,48 @@ impl Command {
|
||||||
"nul byte found in provided data")
|
"nul byte found in provided data")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let possible_paths = self.compute_possible_paths(envp.as_ref());
|
||||||
match self.setup_io(default, true) {
|
match self.setup_io(default, true) {
|
||||||
Ok((_, theirs)) => unsafe { self.do_exec(theirs, envp.as_ref()) },
|
Ok((_, theirs)) => unsafe { self.do_exec(theirs, envp.as_ref(), possible_paths) },
|
||||||
Err(e) => e,
|
Err(e) => e,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn compute_possible_paths(&self, maybe_envp: Option<&CStringArray>) -> Option<Vec<CString>> {
|
||||||
|
let program = self.get_program().as_bytes();
|
||||||
|
if program.contains(&b'/') {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
// Outside the match so we can borrow it for the lifetime of the function.
|
||||||
|
let parent_path = env::var("PATH").ok();
|
||||||
|
let paths = match maybe_envp {
|
||||||
|
Some(envp) => {
|
||||||
|
match envp.get_items().iter().find(|var| var.as_bytes().starts_with(b"PATH=")) {
|
||||||
|
Some(p) => &p.as_bytes()[5..],
|
||||||
|
None => return None,
|
||||||
|
}
|
||||||
|
},
|
||||||
|
// maybe_envp is None if the process isn't changing the parent's env at all.
|
||||||
|
None => {
|
||||||
|
match parent_path.as_ref() {
|
||||||
|
Some(p) => p.as_bytes(),
|
||||||
|
None => return None,
|
||||||
|
}
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
let mut possible_paths = vec![];
|
||||||
|
for path in paths.split(|p| *p == b':') {
|
||||||
|
let mut binary_path = Vec::with_capacity(program.len() + path.len() + 1);
|
||||||
|
binary_path.extend_from_slice(path);
|
||||||
|
binary_path.push(b'/');
|
||||||
|
binary_path.extend_from_slice(program);
|
||||||
|
let c_binary_path = CString::new(binary_path).unwrap();
|
||||||
|
possible_paths.push(c_binary_path);
|
||||||
|
}
|
||||||
|
return Some(possible_paths);
|
||||||
|
}
|
||||||
|
|
||||||
// And at this point we've reached a special time in the life of the
|
// 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
|
// child. The child must now be considered hamstrung and unable to
|
||||||
// do anything other than syscalls really. Consider the following
|
// do anything other than syscalls really. Consider the following
|
||||||
|
@ -152,7 +192,8 @@ impl Command {
|
||||||
unsafe fn do_exec(
|
unsafe fn do_exec(
|
||||||
&mut self,
|
&mut self,
|
||||||
stdio: ChildPipes,
|
stdio: ChildPipes,
|
||||||
maybe_envp: Option<&CStringArray>
|
maybe_envp: Option<&CStringArray>,
|
||||||
|
maybe_possible_paths: Option<Vec<CString>>,
|
||||||
) -> io::Error {
|
) -> io::Error {
|
||||||
use sys::{self, cvt_r};
|
use sys::{self, cvt_r};
|
||||||
|
|
||||||
|
@ -193,9 +234,6 @@ impl Command {
|
||||||
if let Some(ref cwd) = *self.get_cwd() {
|
if let Some(ref cwd) = *self.get_cwd() {
|
||||||
t!(cvt(libc::chdir(cwd.as_ptr())));
|
t!(cvt(libc::chdir(cwd.as_ptr())));
|
||||||
}
|
}
|
||||||
if let Some(envp) = maybe_envp {
|
|
||||||
*sys::os::environ() = envp.as_ptr();
|
|
||||||
}
|
|
||||||
|
|
||||||
// emscripten has no signal support.
|
// emscripten has no signal support.
|
||||||
#[cfg(not(any(target_os = "emscripten")))]
|
#[cfg(not(any(target_os = "emscripten")))]
|
||||||
|
@ -231,8 +269,53 @@ impl Command {
|
||||||
t!(callback());
|
t!(callback());
|
||||||
}
|
}
|
||||||
|
|
||||||
libc::execvp(self.get_argv()[0], self.get_argv().as_ptr());
|
// If the program isn't an absolute path, and our environment contains a PATH var, then we
|
||||||
io::Error::last_os_error()
|
// implement the PATH traversal ourselves so that it honors the child's PATH instead of the
|
||||||
|
// parent's. This mirrors the logic that exists in glibc's execvpe, except using the
|
||||||
|
// child's env to fetch PATH.
|
||||||
|
match maybe_possible_paths {
|
||||||
|
Some(possible_paths) => {
|
||||||
|
let mut pending_error = None;
|
||||||
|
for path in possible_paths {
|
||||||
|
libc::execve(
|
||||||
|
path.as_ptr(),
|
||||||
|
self.get_argv().as_ptr(),
|
||||||
|
maybe_envp.map(|envp| envp.as_ptr()).unwrap_or_else(|| *sys::os::environ())
|
||||||
|
);
|
||||||
|
let err = io::Error::last_os_error();
|
||||||
|
match err.kind() {
|
||||||
|
io::ErrorKind::PermissionDenied => {
|
||||||
|
// If we saw a PermissionDenied, and none of the other entries in
|
||||||
|
// $PATH are successful, then we'll return the first EACCESS we see.
|
||||||
|
if pending_error.is_none() {
|
||||||
|
pending_error = Some(err);
|
||||||
|
}
|
||||||
|
},
|
||||||
|
// Errors which indicate we failed to find a file are ignored and we try
|
||||||
|
// the next entry in the path.
|
||||||
|
io::ErrorKind::NotFound | io::ErrorKind::TimedOut => {
|
||||||
|
continue
|
||||||
|
},
|
||||||
|
// Any other error means we found a file and couldn't execute it.
|
||||||
|
_ => {
|
||||||
|
return err;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if let Some(err) = pending_error {
|
||||||
|
return err;
|
||||||
|
}
|
||||||
|
return io::Error::from_raw_os_error(libc::ENOENT);
|
||||||
|
},
|
||||||
|
_ => {
|
||||||
|
libc::execve(
|
||||||
|
self.get_argv()[0],
|
||||||
|
self.get_argv().as_ptr(),
|
||||||
|
maybe_envp.map(|envp| envp.as_ptr()).unwrap_or_else(|| *sys::os::environ())
|
||||||
|
);
|
||||||
|
return io::Error::last_os_error()
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(not(any(target_os = "macos", target_os = "freebsd",
|
#[cfg(not(any(target_os = "macos", target_os = "freebsd",
|
||||||
|
|
|
@ -48,6 +48,13 @@ fn main() {
|
||||||
println!("passed");
|
println!("passed");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
"exec-test5" => {
|
||||||
|
env::set_var("VARIABLE", "ABC");
|
||||||
|
Command::new("definitely-not-a-real-binary").env("VARIABLE", "XYZ").exec();
|
||||||
|
assert_eq!(env::var("VARIABLE").unwrap(), "ABC");
|
||||||
|
println!("passed");
|
||||||
|
}
|
||||||
|
|
||||||
_ => panic!("unknown argument: {}", arg),
|
_ => panic!("unknown argument: {}", arg),
|
||||||
}
|
}
|
||||||
return
|
return
|
||||||
|
@ -72,4 +79,9 @@ fn main() {
|
||||||
assert!(output.status.success());
|
assert!(output.status.success());
|
||||||
assert!(output.stderr.is_empty());
|
assert!(output.stderr.is_empty());
|
||||||
assert_eq!(output.stdout, b"passed\n");
|
assert_eq!(output.stdout, b"passed\n");
|
||||||
|
|
||||||
|
let output = Command::new(&me).arg("exec-test5").output().unwrap();
|
||||||
|
assert!(output.status.success());
|
||||||
|
assert!(output.stderr.is_empty());
|
||||||
|
assert_eq!(output.stdout, b"passed\n");
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue