From 742089ddae6fbf616988b4c8e749b81226eb01d5 Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Fri, 27 Jan 2017 12:20:08 +0100 Subject: [PATCH 1/3] Check for correct env keys in `Command` Return an error if the key contains an ASCII equals sign (`=`) at the non-first position. --- src/libstd/process.rs | 21 +++++++++ src/libstd/sys/unix/process/process_common.rs | 43 ++++++++++++++++--- .../sys/unix/process/process_fuchsia.rs | 11 ++--- src/libstd/sys/unix/process/process_unix.rs | 11 ++--- 4 files changed, 65 insertions(+), 21 deletions(-) diff --git a/src/libstd/process.rs b/src/libstd/process.rs index 4ff35738b50fb..d34c866e47a5c 100644 --- a/src/libstd/process.rs +++ b/src/libstd/process.rs @@ -1374,6 +1374,27 @@ mod tests { } } + #[test] + fn test_empty_env_key_is_error() { + match env_cmd().env("", "value").spawn() { + Err(e) => assert_eq!(e.kind(), ErrorKind::InvalidInput), + Ok(_) => panic!(), + } + } + + #[test] + fn test_interior_eq_in_env_key_is_error() { + match env_cmd().env("has-some-==s-inside", "value").spawn() { + Err(e) => assert_eq!(e.kind(), ErrorKind::InvalidInput), + Ok(_) => panic!(), + } + } + + #[test] + fn test_leading_eq_key_is_no_error() { + env_cmd().env("=NAME", "value").output().unwrap(); + } + /// Test that process creation flags work by debugging a process. /// Other creation flags make it hard or impossible to detect /// behavioral changes in the process. diff --git a/src/libstd/sys/unix/process/process_common.rs b/src/libstd/sys/unix/process/process_common.rs index a4536520376e8..bfa8e3666b066 100644 --- a/src/libstd/sys/unix/process/process_common.rs +++ b/src/libstd/sys/unix/process/process_common.rs @@ -53,6 +53,7 @@ pub struct Command { uid: Option, gid: Option, saw_nul: bool, + saw_malformed_env_key: bool, closures: Vec io::Result<()> + Send + Sync>>, stdin: Option, stdout: Option, @@ -102,6 +103,7 @@ impl Command { uid: None, gid: None, saw_nul: saw_nul, + saw_malformed_env_key: false, closures: Vec::new(), stdin: None, stdout: None, @@ -127,7 +129,11 @@ impl Command { let mut map = HashMap::new(); let mut envp = Vec::new(); for (k, v) in env::vars_os() { - let s = pair_to_key(&k, &v, &mut self.saw_nul); + let mut saw_nul = false; + let mut saw_malformed_env_key = false; + let s = pair_to_key(&k, &v, &mut saw_nul, &mut saw_malformed_env_key); + assert!(!saw_nul); + assert!(!saw_malformed_env_key); envp.push(s.as_ptr()); map.insert(k, (envp.len() - 1, s)); } @@ -139,7 +145,8 @@ impl Command { } pub fn env(&mut self, key: &OsStr, val: &OsStr) { - let new_key = pair_to_key(key, val, &mut self.saw_nul); + let new_key = pair_to_key(key, val, &mut self.saw_nul, &mut self.saw_malformed_env_key); + let (map, envp) = self.init_env_map(); // If `key` is already present then we just update `envp` in place @@ -162,6 +169,8 @@ impl Command { } pub fn env_remove(&mut self, key: &OsStr) { + pair_to_key(key, OsStr::new(""), &mut self.saw_nul, &mut self.saw_malformed_env_key); + let (map, envp) = self.init_env_map(); // If we actually ended up removing a key, then we need to update the @@ -193,9 +202,6 @@ impl Command { self.gid = Some(id); } - pub fn saw_nul(&self) -> bool { - self.saw_nul - } pub fn get_envp(&self) -> &Option> { &self.envp } @@ -237,6 +243,18 @@ impl Command { self.stderr = Some(stderr); } + pub fn check_malformed(&self) -> io::Result<()> { + if self.saw_nul { + return Err(io::Error::new(io::ErrorKind::InvalidInput, + "nul byte found in provided data")); + } + if self.saw_malformed_env_key { + return Err(io::Error::new(io::ErrorKind::InvalidInput, + "malformed env key in provided data")); + } + Ok(()) + } + pub fn setup_io(&self, default: Stdio, needs_stdin: bool) -> io::Result<(StdioPipes, ChildPipes)> { let null = Stdio::Null; @@ -261,6 +279,13 @@ impl Command { } } +fn check_env_key(s: &OsStr, saw_malformed: &mut bool) { + let bytes = s.as_bytes(); + if bytes.is_empty() || bytes[1..].contains(&b'=') { + *saw_malformed = true; + } +} + fn os2c(s: &OsStr, saw_nul: &mut bool) -> CString { CString::new(s.as_bytes()).unwrap_or_else(|_e| { *saw_nul = true; @@ -325,7 +350,11 @@ impl ChildStdio { } } -fn pair_to_key(key: &OsStr, value: &OsStr, saw_nul: &mut bool) -> CString { +fn pair_to_key(key: &OsStr, value: &OsStr, saw_nul: &mut bool, saw_malformed: &mut bool) + -> CString +{ + check_env_key(key, saw_malformed); + let (key, value) = (key.as_bytes(), value.as_bytes()); let mut v = Vec::with_capacity(key.len() + value.len() + 1); v.extend(key); @@ -333,7 +362,7 @@ fn pair_to_key(key: &OsStr, value: &OsStr, saw_nul: &mut bool) -> CString { v.extend(value); CString::new(v).unwrap_or_else(|_e| { *saw_nul = true; - CString::new("foo=bar").unwrap() + CString::new("").unwrap() }) } diff --git a/src/libstd/sys/unix/process/process_fuchsia.rs b/src/libstd/sys/unix/process/process_fuchsia.rs index 0bb2e0c1a83d4..4b0c9848d3403 100644 --- a/src/libstd/sys/unix/process/process_fuchsia.rs +++ b/src/libstd/sys/unix/process/process_fuchsia.rs @@ -23,10 +23,7 @@ use sys::process::process_common::*; impl Command { pub fn spawn(&mut self, default: Stdio, needs_stdin: bool) -> io::Result<(Process, StdioPipes)> { - if self.saw_nul() { - return Err(io::Error::new(io::ErrorKind::InvalidInput, - "nul byte found in provided data")); - } + self.check_malformed()?; let (ours, theirs) = self.setup_io(default, needs_stdin)?; @@ -36,9 +33,9 @@ impl Command { } pub fn exec(&mut self, default: Stdio) -> io::Error { - if self.saw_nul() { - return io::Error::new(io::ErrorKind::InvalidInput, - "nul byte found in provided data") + match self.check_malformed() { + Ok(()) => {}, + Err(e) => return e, } match self.setup_io(default, true) { diff --git a/src/libstd/sys/unix/process/process_unix.rs b/src/libstd/sys/unix/process/process_unix.rs index bbc987209e300..14549bf01e7ad 100644 --- a/src/libstd/sys/unix/process/process_unix.rs +++ b/src/libstd/sys/unix/process/process_unix.rs @@ -27,10 +27,7 @@ impl Command { const CLOEXEC_MSG_FOOTER: &'static [u8] = b"NOEX"; - if self.saw_nul() { - return Err(io::Error::new(ErrorKind::InvalidInput, - "nul byte found in provided data")); - } + self.check_malformed()?; let (ours, theirs) = self.setup_io(default, needs_stdin)?; let (input, output) = sys::pipe::anon_pipe()?; @@ -100,9 +97,9 @@ impl Command { } pub fn exec(&mut self, default: Stdio) -> io::Error { - if self.saw_nul() { - return io::Error::new(ErrorKind::InvalidInput, - "nul byte found in provided data") + match self.check_malformed() { + Ok(()) => {}, + Err(e) => return e, } match self.setup_io(default, true) { From 314649d231e54005a665c7127f2dd1dd78993631 Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Wed, 15 Feb 2017 18:56:31 +0100 Subject: [PATCH 2/3] Check Windows env keys for interior `=` --- src/libstd/sys/windows/process.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/libstd/sys/windows/process.rs b/src/libstd/sys/windows/process.rs index 1afb3728c9d72..6da03e496bda3 100644 --- a/src/libstd/sys/windows/process.rs +++ b/src/libstd/sys/windows/process.rs @@ -49,6 +49,15 @@ fn ensure_no_nuls>(str: T) -> io::Result { } } +fn ensure_valid_env_key>(str: T) -> io::Result { + if str.as_ref().is_empty() || str.as_ref().encode_wide().skip(1).any(|b| b == b'=' as u16) { + Err(io::Error::new(io::ErrorKind::InvalidInput, + "malformed env key in provided data")) + } else { + ensure_no_nuls(str) + } +} + pub struct Command { program: OsString, args: Vec, @@ -479,7 +488,7 @@ fn make_envp(env: Option<&collections::HashMap>) let mut blk = Vec::new(); for pair in env { - blk.extend(ensure_no_nuls(pair.0)?.encode_wide()); + blk.extend(ensure_valid_env_key(pair.0)?.encode_wide()); blk.push('=' as u16); blk.extend(ensure_no_nuls(pair.1)?.encode_wide()); blk.push(0); From 58f7804bb3491eba30026f1957fb31807db691eb Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Tue, 7 Mar 2017 23:36:46 +0100 Subject: [PATCH 3/3] Add println!-debugging --- src/libstd/sys/unix/process/process_common.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libstd/sys/unix/process/process_common.rs b/src/libstd/sys/unix/process/process_common.rs index bfa8e3666b066..8ffbb0e9dd567 100644 --- a/src/libstd/sys/unix/process/process_common.rs +++ b/src/libstd/sys/unix/process/process_common.rs @@ -146,6 +146,9 @@ impl Command { pub fn env(&mut self, key: &OsStr, val: &OsStr) { let new_key = pair_to_key(key, val, &mut self.saw_nul, &mut self.saw_malformed_env_key); + if self.saw_malformed_env_key { + println!("{:?} {:?}", key, val); + } let (map, envp) = self.init_env_map(); @@ -170,6 +173,9 @@ impl Command { pub fn env_remove(&mut self, key: &OsStr) { pair_to_key(key, OsStr::new(""), &mut self.saw_nul, &mut self.saw_malformed_env_key); + if self.saw_malformed_env_key { + println!("{:?}", key); + } let (map, envp) = self.init_env_map();