Skip to content

Commit 1205733

Browse files
bjorn3bjorn3
authored
Allow non-UTF-8 arguments in sudo (#1413)
2 parents 74e0db4 + 0bcb98d commit 1205733

File tree

12 files changed

+166
-86
lines changed

12 files changed

+166
-86
lines changed

‎src/common/command.rs‎

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
use std::{
2+
ffi::{OsStr, OsString},
23
fmt::Display,
4+
os::unix::ffi::OsStringExt,
35
path::{Path, PathBuf},
46
};
57

8+
use crate::common::DisplayOsStr;
69
use crate::system::escape_os_str_lossy;
710

811
use super::resolve::{canonicalize, resolve_path};
@@ -11,7 +14,7 @@ use super::resolve::{canonicalize, resolve_path};
1114
#[cfg_attr(test, derive(PartialEq))]
1215
pub struct CommandAndArguments {
1316
pub(crate) command: PathBuf,
14-
pub(crate) arguments: Vec<String>,
17+
pub(crate) arguments: Vec<OsString>,
1518
pub(crate) resolved: bool,
1619
pub(crate) arg0: Option<PathBuf>,
1720
}
@@ -22,28 +25,35 @@ impl Display for CommandAndArguments {
2225
let args = self
2326
.arguments
2427
.iter()
25-
.map(|a| a.escape_default().collect::<String>())
28+
.map(|a| {
29+
DisplayOsStr(a)
30+
.to_string()
31+
.escape_default()
32+
.collect::<String>()
33+
})
2634
.collect::<Vec<_>>()
2735
.join(" ");
2836
write!(f, "{cmd} {args}")
2937
}
3038
}
3139

3240
// when -i and -s are used, the arguments given to sudo are escaped "except for alphanumerics, underscores, hyphens, and dollar signs."
33-
fn escaped(arguments: Vec<String>) -> String {
41+
fn escaped(arguments: Vec<OsString>) -> OsString {
3442
arguments
3543
.into_iter()
3644
.map(|arg| {
37-
arg.chars()
38-
.map(|c| match c {
39-
'_' | '-' | '$' => c.to_string(),
40-
c if c.is_alphanumeric() => c.to_string(),
41-
_ => ['\\', c].iter().collect(),
42-
})
43-
.collect()
45+
let mut escaped_arg = Vec::new();
46+
for c in arg.into_encoded_bytes() {
47+
match c {
48+
b'_' | b'-' | b'$' => escaped_arg.push(c),
49+
c if c.is_ascii_alphanumeric() => escaped_arg.push(c),
50+
_ => escaped_arg.extend_from_slice(&[b'\\', c]),
51+
}
52+
}
53+
OsString::from_vec(escaped_arg)
4454
})
45-
.collect::<Vec<String>>()
46-
.join(" ")
55+
.collect::<Vec<OsString>>()
56+
.join(OsStr::new(" "))
4757
}
4858

4959
//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 {
5363
}
5464

5565
impl CommandAndArguments {
56-
pub fn build_from_args(shell: Option<PathBuf>, mut arguments: Vec<String>, path: &str) -> Self {
66+
pub fn build_from_args(
67+
shell: Option<PathBuf>,
68+
mut arguments: Vec<OsString>,
69+
path: &str,
70+
) -> Self {
5771
let mut resolved = true;
5872
let mut command;
5973
let mut arg0 = None;
6074
if let Some(chosen_shell) = shell {
6175
command = chosen_shell;
6276
if !arguments.is_empty() {
63-
arguments = vec!["-c".to_string(), escaped(arguments)]
77+
arguments = vec!["-c".into(), escaped(arguments)]
6478
}
6579
} else {
6680
command = arguments.first().map(|s| s.into()).unwrap_or_default();
@@ -97,15 +111,14 @@ impl CommandAndArguments {
97111

98112
#[cfg(test)]
99113
mod test {
114+
use std::ffi::OsString;
115+
100116
use super::{CommandAndArguments, escaped};
101117

102118
#[test]
103119
fn test_escaped() {
104120
let test = |src: &[&str], target: &str| {
105-
assert_eq!(
106-
&escaped(src.iter().map(|s| s.to_string()).collect()),
107-
target
108-
);
121+
assert_eq!(&escaped(src.iter().map(OsString::from).collect()), target);
109122
};
110123
test(&["a", "b", "c"], "a b c");
111124
test(&["a", "b c"], "a b\\ c");

‎src/common/context.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use std::env;
2+
use std::ffi::OsString;
23

34
use crate::common::{Error, HARDENED_ENUM_VALUE_0, HARDENED_ENUM_VALUE_1, HARDENED_ENUM_VALUE_2};
45
use crate::exec::RunOptions;
@@ -137,8 +138,8 @@ impl Context {
137138
// by the policy to edit that file. this is to prevent leaking information.
138139
let arguments = resolved_args
139140
.map(|arg| match arg {
140-
Ok(arg) => arg,
141-
Err(arg) => arg.to_owned(),
141+
Ok(arg) => OsString::from(arg),
142+
Err(arg) => OsString::from(arg),
142143
})
143144
.collect();
144145

‎src/common/mod.rs‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
#![forbid(unsafe_code)]
22

3+
use std::ffi::OsStr;
4+
use std::fmt::{self, Write};
5+
36
pub use command::CommandAndArguments;
47
pub use context::Context;
58
pub use error::Error;
@@ -22,3 +25,24 @@ pub const HARDENED_ENUM_VALUE_1: u32 = 0xad5d6da; // 101011010101110101101101101
2225
pub const HARDENED_ENUM_VALUE_2: u32 = 0x69d61fc8; // 1101001110101100001111111001000
2326
pub const HARDENED_ENUM_VALUE_3: u32 = 0x1629e037; // 0010110001010011110000000110111
2427
pub const HARDENED_ENUM_VALUE_4: u32 = 0x1fc8d3ac; // 11111110010001101001110101100
28+
29+
// FIXME replace with OsStr::display() once our MSRV is 1.87
30+
pub(crate) struct DisplayOsStr<'a>(pub(crate) &'a OsStr);
31+
32+
impl fmt::Display for DisplayOsStr<'_> {
33+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
34+
for chunk in self.0.as_encoded_bytes().utf8_chunks() {
35+
let valid = chunk.valid();
36+
// If we successfully decoded the whole chunk as a valid string then
37+
// we can return a direct formatting of the string which will also
38+
// respect various formatting flags if possible.
39+
if chunk.invalid().is_empty() {
40+
return valid.fmt(f);
41+
}
42+
43+
f.write_str(valid)?;
44+
f.write_char(char::REPLACEMENT_CHARACTER)?;
45+
}
46+
Ok(())
47+
}
48+
}

‎src/exec/mod.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ mod use_pty;
88
use std::{
99
borrow::Cow,
1010
env,
11-
ffi::{OsStr, c_int},
11+
ffi::{OsStr, OsString, c_int},
1212
io,
1313
os::unix::ffi::OsStrExt,
1414
os::unix::process::CommandExt,
@@ -62,7 +62,7 @@ pub enum Umask {
6262

6363
pub struct RunOptions<'a> {
6464
pub command: &'a Path,
65-
pub arguments: &'a [String],
65+
pub arguments: &'a [OsString],
6666
pub arg0: Option<&'a Path>,
6767
pub chdir: Option<PathBuf>,
6868
pub is_login: bool,

‎src/su/cli.rs‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::{mem, path::PathBuf};
1+
use std::{ffi::OsString, mem, path::PathBuf};
22

33
use crate::common::SudoString;
44

@@ -49,7 +49,7 @@ impl TryFrom<SuOptions> for SuVersionOptions {
4949
#[cfg_attr(test, derive(PartialEq))]
5050
pub struct SuRunOptions {
5151
// -c
52-
pub command: Option<String>,
52+
pub command: Option<OsString>,
5353
// -g
5454
pub group: Vec<SudoString>,
5555
// -l
@@ -64,7 +64,7 @@ pub struct SuRunOptions {
6464
pub whitelist_environment: Vec<String>,
6565

6666
pub user: SudoString,
67-
pub arguments: Vec<String>,
67+
pub arguments: Vec<OsString>,
6868
}
6969

7070
#[cfg(test)]
@@ -106,7 +106,7 @@ impl TryFrom<SuOptions> for SuRunOptions {
106106
} else {
107107
positional_args.remove(0)
108108
};
109-
let arguments = positional_args;
109+
let arguments = positional_args.into_iter().map(OsString::from).collect();
110110

111111
Ok(Self {
112112
command,
@@ -182,7 +182,7 @@ impl<T> IsAbsent for Vec<T> {
182182
#[derive(Debug, Default, PartialEq)]
183183
pub(super) struct SuOptions {
184184
// -c
185-
command: Option<String>,
185+
command: Option<OsString>,
186186
// -g
187187
group: Vec<SudoString>,
188188
// -h
@@ -222,7 +222,7 @@ impl SuOptions {
222222
takes_argument: true,
223223
set: |sudo_options, argument| {
224224
if argument.is_some() {
225-
sudo_options.command = argument;
225+
sudo_options.command = argument.map(OsString::from);
226226
Ok(())
227227
} else {
228228
Err("no command provided".into())
@@ -555,7 +555,7 @@ mod tests {
555555
fn it_parses_arguments() {
556556
let expected = SuAction::Run(SuRunOptions {
557557
user: "ferris".into(),
558-
arguments: vec!["script.sh".to_string()],
558+
arguments: vec!["script.sh".into()],
559559
..<_>::default()
560560
});
561561

@@ -565,7 +565,7 @@ mod tests {
565565
#[test]
566566
fn it_parses_command() {
567567
let expected = SuAction::Run(SuRunOptions {
568-
command: Some("'echo hi'".to_string()),
568+
command: Some("'echo hi'".into()),
569569
..<_>::default()
570570
});
571571
assert_eq!(expected, parse(&["-c", "'echo hi'"]));
@@ -574,7 +574,7 @@ mod tests {
574574
assert_eq!(expected, parse(&["--command='echo hi'"]));
575575

576576
let expected = SuAction::Run(SuRunOptions {
577-
command: Some("env".to_string()),
577+
command: Some("env".into()),
578578
..<_>::default()
579579
});
580580
assert_eq!(expected, parse(&["-c", "env"]));
@@ -705,7 +705,7 @@ mod tests {
705705
#[test]
706706
fn flags_after_dash() {
707707
let expected = SuAction::Run(SuRunOptions {
708-
command: Some("echo".to_string()),
708+
command: Some("echo".into()),
709709
login: true,
710710
..<_>::default()
711711
});
@@ -716,7 +716,7 @@ mod tests {
716716
fn only_positional_args_after_dashdash() {
717717
let expected = SuAction::Run(SuRunOptions {
718718
user: "ferris".into(),
719-
arguments: vec!["-c".to_string(), "echo".to_string()],
719+
arguments: vec!["-c".into(), "echo".into()],
720720
..<_>::default()
721721
});
722722
assert_eq!(expected, parse(&["--", "ferris", "-c", "echo"]));

‎src/su/context.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ const PATH_DEFAULT_ROOT: &str = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/b
2626
#[derive(Debug)]
2727
pub(crate) struct SuContext {
2828
command: PathBuf,
29-
arguments: Vec<String>,
29+
arguments: Vec<OsString>,
3030
pub(crate) options: SuRunOptions,
3131
pub(crate) environment: Environment,
3232
pub(crate) user: User,
@@ -158,7 +158,7 @@ impl SuContext {
158158

159159
// pass command to shell
160160
let arguments = if let Some(command) = &options.command {
161-
vec!["-c".to_owned(), command.to_owned()]
161+
vec!["-c".into(), command.into()]
162162
} else {
163163
options.arguments.clone()
164164
};

0 commit comments

Comments
 (0)