Skip to content

Commit 57d3fce

Browse files
authored
chroot: use execvp directly instead of process::Command (#9013)
* chroot: exec command with Command::exec and map errors * test(chroot): add error handling and UID/GID retention tests * chore(cspell): add noexec to jargon wordlist
1 parent 6f37a24 commit 57d3fce

File tree

3 files changed

+80
-23
lines changed

3 files changed

+80
-23
lines changed

.vscode/cspell.dictionaries/jargon.wordlist.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ nocache
9797
nocreat
9898
noctty
9999
noerror
100+
noexec
100101
nofollow
101102
nolinks
102103
nonblock

src/uu/chroot/src/chroot.rs

Lines changed: 11 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,13 @@ mod error;
99
use crate::error::ChrootError;
1010
use clap::{Arg, ArgAction, Command};
1111
use std::ffi::CString;
12-
use std::io::Error;
12+
use std::io::{Error, ErrorKind};
1313
use std::os::unix::prelude::OsStrExt;
14+
use std::os::unix::process::CommandExt;
1415
use std::path::{Path, PathBuf};
1516
use std::process;
1617
use uucore::entries::{Locate, Passwd, grp2gid, usr2uid};
17-
use uucore::error::{UResult, UUsageError, set_exit_code};
18+
use uucore::error::{UResult, UUsageError};
1819
use uucore::fs::{MissingHandling, ResolveMode, canonicalize};
1920
use uucore::libc::{self, chroot, setgid, setgroups, setuid};
2021
use uucore::{format_usage, show};
@@ -205,33 +206,20 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
205206

206207
assert!(!command.is_empty());
207208
let chroot_command = command[0];
208-
let chroot_args = &command[1..];
209209

210210
// NOTE: Tests can only trigger code beyond this point if they're invoked with root permissions
211211
set_context(&options)?;
212212

213-
let pstatus = match process::Command::new(chroot_command)
214-
.args(chroot_args)
215-
.status()
216-
{
217-
Ok(status) => status,
218-
Err(e) => {
219-
return Err(if e.kind() == std::io::ErrorKind::NotFound {
220-
ChrootError::CommandNotFound(command[0].to_string(), e)
221-
} else {
222-
ChrootError::CommandFailed(command[0].to_string(), e)
223-
}
224-
.into());
225-
}
226-
};
213+
let err = process::Command::new(chroot_command)
214+
.args(&command[1..])
215+
.exec();
227216

228-
let code = if pstatus.success() {
229-
0
217+
Err(if err.kind() == ErrorKind::NotFound {
218+
ChrootError::CommandNotFound(chroot_command.to_owned(), err)
230219
} else {
231-
pstatus.code().unwrap_or(-1)
232-
};
233-
set_exit_code(code);
234-
Ok(())
220+
ChrootError::CommandFailed(chroot_command.to_owned(), err)
221+
}
222+
.into())
235223
}
236224

237225
pub fn uu_app() -> Command {

tests/by-util/test_chroot.rs

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,53 @@ fn test_default_shell() {
188188
}
189189
}
190190

191+
#[test]
192+
fn test_chroot_command_not_found_error() {
193+
let ts = TestScenario::new(util_name!());
194+
let at = &ts.fixtures;
195+
196+
let dir = "CHROOT_DIR";
197+
at.mkdir(dir);
198+
199+
let missing = "definitely_missing_command";
200+
201+
if let Ok(result) = run_ucmd_as_root(&ts, &[dir, missing]) {
202+
result
203+
.failure()
204+
.code_is(127)
205+
.stderr_contains(format!("failed to run command '{missing}'"))
206+
.stderr_contains("No such file or directory");
207+
} else {
208+
print!("Test skipped; requires root user");
209+
}
210+
}
211+
212+
#[test]
213+
fn test_chroot_command_permission_denied_error() {
214+
let ts = TestScenario::new(util_name!());
215+
let at = &ts.fixtures;
216+
217+
let dir = "CHROOT_DIR";
218+
at.mkdir(dir);
219+
220+
let script_path = format!("{dir}/noexec.sh");
221+
at.write(&script_path, "#!/bin/sh\necho unreachable\n");
222+
#[cfg(not(windows))]
223+
{
224+
at.set_mode(&script_path, 0o644);
225+
}
226+
227+
if let Ok(result) = run_ucmd_as_root(&ts, &[dir, "/noexec.sh"]) {
228+
result
229+
.failure()
230+
.code_is(126)
231+
.stderr_contains("failed to run command '/noexec.sh'")
232+
.stderr_contains("Permission denied");
233+
} else {
234+
print!("Test skipped; requires root user");
235+
}
236+
}
237+
191238
#[test]
192239
fn test_chroot() {
193240
let ts = TestScenario::new(util_name!());
@@ -208,6 +255,27 @@ fn test_chroot() {
208255
}
209256
}
210257

258+
#[test]
259+
fn test_chroot_retains_uid_gid() {
260+
let ts = TestScenario::new(util_name!());
261+
let at = &ts.fixtures;
262+
263+
let dir = "CHROOT_DIR";
264+
at.mkdir(dir);
265+
266+
if let Ok(result) = run_ucmd_as_root(&ts, &[dir, "id", "-u"]) {
267+
result.success().no_stderr().stdout_is("0");
268+
} else {
269+
print!("Test skipped; requires root user");
270+
}
271+
272+
if let Ok(result) = run_ucmd_as_root(&ts, &[dir, "id", "-g"]) {
273+
result.success().no_stderr().stdout_is("0");
274+
} else {
275+
print!("Test skipped; requires root user");
276+
}
277+
}
278+
211279
#[test]
212280
fn test_chroot_skip_chdir_not_root() {
213281
let (at, mut ucmd) = at_and_ucmd!();

0 commit comments

Comments
 (0)