Allow non-UTF-8 arguments in sudo

src/common/command.rs

@@ -1,8 +1,11 @@
 use std::{
+    ffi::{OsStr, OsString},
     fmt::Display,
+    os::unix::ffi::OsStringExt,
     path::{Path, PathBuf},
 };
 
+use crate::common::DisplayOsStr;
 use crate::system::escape_os_str_lossy;
 
 use super::resolve::{canonicalize, resolve_path};
@@ -11,7 +14,7 @@ use super::resolve::{canonicalize, resolve_path};
 #[cfg_attr(test, derive(PartialEq))]
 pub struct CommandAndArguments {
     pub(crate) command: PathBuf,
-    pub(crate) arguments: Vec<String>,
+    pub(crate) arguments: Vec<OsString>,
     pub(crate) resolved: bool,
     pub(crate) arg0: Option<PathBuf>,
 }
@@ -22,28 +25,35 @@ impl Display for CommandAndArguments {
         let args = self
             .arguments
             .iter()
-            .map(|a| a.escape_default().collect::<String>())
+            .map(|a| {
+                DisplayOsStr(a)
+                    .to_string()
+                    .escape_default()
+                    .collect::<String>()
+            })
             .collect::<Vec<_>>()
             .join(" ");
         write!(f, "{cmd} {args}")
     }
 }
 
 // when -i and -s are used, the arguments given to sudo are escaped "except for alphanumerics, underscores, hyphens, and dollar signs."
-fn escaped(arguments: Vec<String>) -> String {
+fn escaped(arguments: Vec<OsString>) -> OsString {
     arguments
         .into_iter()
         .map(|arg| {
-            arg.chars()
-                .map(|c| match c {
-                    '_' | '-' | '$' => c.to_string(),
-                    c if c.is_alphanumeric() => c.to_string(),
-                    _ => ['\\', c].iter().collect(),
-                })
-                .collect()
+            let mut escaped_arg = Vec::new();
+            for c in arg.into_encoded_bytes() {
+                match c {
+                    b'_' | b'-' | b'$' => escaped_arg.push(c),
+                    c if c.is_ascii_alphanumeric() => escaped_arg.push(c),
+                    _ => escaped_arg.extend_from_slice(&[b'\\', c]),
+                }
+            }
+            OsString::from_vec(escaped_arg)
         })
-        .collect::<Vec<String>>()
-        .join(" ")
+        .collect::<Vec<OsString>>()
+        .join(OsStr::new(" "))
 }
 
 //checks whether the Path is actually describing a qualified path (i.e. contains "/")
@@ -53,14 +63,18 @@ fn is_qualified(path: impl AsRef<Path>) -> bool {
 }
 
 impl CommandAndArguments {
-    pub fn build_from_args(shell: Option<PathBuf>, mut arguments: Vec<String>, path: &str) -> Self {
+    pub fn build_from_args(
+        shell: Option<PathBuf>,
+        mut arguments: Vec<OsString>,
+        path: &str,
+    ) -> Self {
         let mut resolved = true;
         let mut command;
         let mut arg0 = None;
         if let Some(chosen_shell) = shell {
             command = chosen_shell;
             if !arguments.is_empty() {
-                arguments = vec!["-c".to_string(), escaped(arguments)]
+                arguments = vec!["-c".into(), escaped(arguments)]
             }
         } else {
             command = arguments.first().map(|s| s.into()).unwrap_or_default();
@@ -97,15 +111,14 @@ impl CommandAndArguments {
 
 #[cfg(test)]
 mod test {
+    use std::ffi::OsString;
+
     use super::{CommandAndArguments, escaped};
 
     #[test]
     fn test_escaped() {
         let test = |src: &[&str], target: &str| {
-            assert_eq!(
-                &escaped(src.iter().map(|s| s.to_string()).collect()),
-                target
-            );
+            assert_eq!(&escaped(src.iter().map(OsString::from).collect()), target);
         };
         test(&["a", "b", "c"], "a b c");
         test(&["a", "b c"], "a b\\ c");

src/common/context.rs

@@ -1,4 +1,5 @@
 use std::env;
+use std::ffi::OsString;
 
 use crate::common::{Error, HARDENED_ENUM_VALUE_0, HARDENED_ENUM_VALUE_1, HARDENED_ENUM_VALUE_2};
 use crate::exec::RunOptions;
@@ -137,8 +138,8 @@ impl Context {
         // by the policy to edit that file. this is to prevent leaking information.
         let arguments = resolved_args
             .map(|arg| match arg {
-                Ok(arg) => arg,
-                Err(arg) => arg.to_owned(),
+                Ok(arg) => OsString::from(arg),
+                Err(arg) => OsString::from(arg),
             })
             .collect();
 

src/common/mod.rs

@@ -1,5 +1,8 @@
 #![forbid(unsafe_code)]
 
+use std::ffi::OsStr;
+use std::fmt::{self, Write};
+
 pub use command::CommandAndArguments;
 pub use context::Context;
 pub use error::Error;
@@ -22,3 +25,24 @@ pub const HARDENED_ENUM_VALUE_1: u32 = 0xad5d6da; // 101011010101110101101101101
 pub const HARDENED_ENUM_VALUE_2: u32 = 0x69d61fc8; // 1101001110101100001111111001000
 pub const HARDENED_ENUM_VALUE_3: u32 = 0x1629e037; // 0010110001010011110000000110111
 pub const HARDENED_ENUM_VALUE_4: u32 = 0x1fc8d3ac; // 11111110010001101001110101100
+
+// FIXME replace with OsStr::display() once our MSRV is 1.87
+pub(crate) struct DisplayOsStr<'a>(pub(crate) &'a OsStr);
+
+impl fmt::Display for DisplayOsStr<'_> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        for chunk in self.0.as_encoded_bytes().utf8_chunks() {
+            let valid = chunk.valid();
+            // If we successfully decoded the whole chunk as a valid string then
+            // we can return a direct formatting of the string which will also
+            // respect various formatting flags if possible.
+            if chunk.invalid().is_empty() {
+                return valid.fmt(f);
+            }
+
+            f.write_str(valid)?;
+            f.write_char(char::REPLACEMENT_CHARACTER)?;
+        }
+        Ok(())
+    }
+}

src/exec/mod.rs

@@ -8,7 +8,7 @@ mod use_pty;
 use std::{
     borrow::Cow,
     env,
-    ffi::{OsStr, c_int},
+    ffi::{OsStr, OsString, c_int},
     io,
     os::unix::ffi::OsStrExt,
     os::unix::process::CommandExt,
@@ -62,7 +62,7 @@ pub enum Umask {
 
 pub struct RunOptions<'a> {
     pub command: &'a Path,
-    pub arguments: &'a [String],
+    pub arguments: &'a [OsString],
     pub arg0: Option<&'a Path>,
     pub chdir: Option<PathBuf>,
     pub is_login: bool,

src/su/cli.rs

@@ -1,4 +1,4 @@
-use std::{mem, path::PathBuf};
+use std::{ffi::OsString, mem, path::PathBuf};
 
 use crate::common::SudoString;
 
@@ -49,7 +49,7 @@ impl TryFrom<SuOptions> for SuVersionOptions {
 #[cfg_attr(test, derive(PartialEq))]
 pub struct SuRunOptions {
     // -c
-    pub command: Option<String>,
+    pub command: Option<OsString>,
     // -g
     pub group: Vec<SudoString>,
     // -l
@@ -64,7 +64,7 @@ pub struct SuRunOptions {
     pub whitelist_environment: Vec<String>,
 
     pub user: SudoString,
-    pub arguments: Vec<String>,
+    pub arguments: Vec<OsString>,
 }
 
 #[cfg(test)]
@@ -106,7 +106,7 @@ impl TryFrom<SuOptions> for SuRunOptions {
         } else {
             positional_args.remove(0)
         };
-        let arguments = positional_args;
+        let arguments = positional_args.into_iter().map(OsString::from).collect();
 
         Ok(Self {
             command,
@@ -182,7 +182,7 @@ impl<T> IsAbsent for Vec<T> {
 #[derive(Debug, Default, PartialEq)]
 pub(super) struct SuOptions {
     // -c
-    command: Option<String>,
+    command: Option<OsString>,
     // -g
     group: Vec<SudoString>,
     // -h
@@ -222,7 +222,7 @@ impl SuOptions {
             takes_argument: true,
             set: |sudo_options, argument| {
                 if argument.is_some() {
-                    sudo_options.command = argument;
+                    sudo_options.command = argument.map(OsString::from);
                     Ok(())
                 } else {
                     Err("no command provided".into())
@@ -555,7 +555,7 @@ mod tests {
     fn it_parses_arguments() {
         let expected = SuAction::Run(SuRunOptions {
             user: "ferris".into(),
-            arguments: vec!["script.sh".to_string()],
+            arguments: vec!["script.sh".into()],
             ..<_>::default()
         });
 
@@ -565,7 +565,7 @@ mod tests {
     #[test]
     fn it_parses_command() {
         let expected = SuAction::Run(SuRunOptions {
-            command: Some("'echo hi'".to_string()),
+            command: Some("'echo hi'".into()),
             ..<_>::default()
         });
         assert_eq!(expected, parse(&["-c", "'echo hi'"]));
@@ -574,7 +574,7 @@ mod tests {
         assert_eq!(expected, parse(&["--command='echo hi'"]));
 
         let expected = SuAction::Run(SuRunOptions {
-            command: Some("env".to_string()),
+            command: Some("env".into()),
             ..<_>::default()
         });
         assert_eq!(expected, parse(&["-c", "env"]));
@@ -705,7 +705,7 @@ mod tests {
     #[test]
     fn flags_after_dash() {
         let expected = SuAction::Run(SuRunOptions {
-            command: Some("echo".to_string()),
+            command: Some("echo".into()),
             login: true,
             ..<_>::default()
         });
@@ -716,7 +716,7 @@ mod tests {
     fn only_positional_args_after_dashdash() {
         let expected = SuAction::Run(SuRunOptions {
             user: "ferris".into(),
-            arguments: vec!["-c".to_string(), "echo".to_string()],
+            arguments: vec!["-c".into(), "echo".into()],
             ..<_>::default()
         });
         assert_eq!(expected, parse(&["--", "ferris", "-c", "echo"]));

src/su/context.rs

@@ -26,7 +26,7 @@ const PATH_DEFAULT_ROOT: &str = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/b
 #[derive(Debug)]
 pub(crate) struct SuContext {
     command: PathBuf,
-    arguments: Vec<String>,
+    arguments: Vec<OsString>,
     pub(crate) options: SuRunOptions,
     pub(crate) environment: Environment,
     pub(crate) user: User,
@@ -158,7 +158,7 @@ impl SuContext {
 
         // pass command to shell
         let arguments = if let Some(command) = &options.command {
-            vec!["-c".to_owned(), command.to_owned()]
+            vec!["-c".into(), command.into()]
         } else {
             options.arguments.clone()
         };