From d768b19f2b9a74e2941f4a4cca87d22992863377 Mon Sep 17 00:00:00 2001
From: Sylvestre Ledru <sylvestre@debian.org>
Date: Fri, 30 Sep 2022 17:33:52 -1000
Subject: [PATCH] chroot: improve support of --skip-chdir

Should unbreak tests/misc/chroot-fail.sh
---
 src/uu/chroot/src/chroot.rs  | 17 +++++++++++++-
 tests/by-util/test_chroot.rs | 43 ++++++++++++++++++++++++------------
 2 files changed, 45 insertions(+), 15 deletions(-)

diff --git a/src/uu/chroot/src/chroot.rs b/src/uu/chroot/src/chroot.rs
index f4e86938a..1414da666 100644
--- a/src/uu/chroot/src/chroot.rs
+++ b/src/uu/chroot/src/chroot.rs
@@ -16,7 +16,8 @@ use std::io::Error;
 use std::os::unix::prelude::OsStrExt;
 use std::path::Path;
 use std::process;
-use uucore::error::{set_exit_code, UClapError, UResult};
+use uucore::error::{set_exit_code, UClapError, UResult, UUsageError};
+use uucore::fs::{canonicalize, MissingHandling, ResolveMode};
 use uucore::libc::{self, chroot, setgid, setgroups, setuid};
 use uucore::{entries, format_usage};
 
@@ -48,6 +49,20 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         None => return Err(ChrootError::MissingNewRoot.into()),
     };
 
+    let skip_chdir = matches.contains_id(options::SKIP_CHDIR);
+    // We are resolving the path in case it is a symlink or /. or /../
+    if skip_chdir
+        && canonicalize(newroot, MissingHandling::Normal, ResolveMode::Logical)
+            .unwrap()
+            .to_str()
+            != Some("/")
+    {
+        return Err(UUsageError::new(
+            125,
+            "option --skip-chdir only permitted if NEWROOT is old '/'",
+        ));
+    }
+
     if !newroot.is_dir() {
         return Err(ChrootError::NoSuchDirectory(format!("{}", newroot.display())).into());
     }
diff --git a/tests/by-util/test_chroot.rs b/tests/by-util/test_chroot.rs
index ca04fd984..b53ca4e5b 100644
--- a/tests/by-util/test_chroot.rs
+++ b/tests/by-util/test_chroot.rs
@@ -1,4 +1,4 @@
-// spell-checker:ignore (words) araba newroot userspec chdir pwd's
+// spell-checker:ignore (words) araba newroot userspec chdir pwd's isroot
 
 use crate::common::util::*;
 
@@ -145,22 +145,37 @@ fn test_chroot() {
     }
 }
 
+#[test]
+fn test_chroot_skip_chdir_not_root() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    let dir = "foobar";
+    at.mkdir(dir);
+
+    ucmd.arg("--skip-chdir")
+        .arg(dir)
+        .fails()
+        .stderr_contains("chroot: option --skip-chdir only permitted if NEWROOT is old '/'")
+        .code_is(125);
+}
+
 #[test]
 fn test_chroot_skip_chdir() {
     let ts = TestScenario::new(util_name!());
-    let at = &ts.fixtures;
-
-    let dir = "CHROOT_DIR";
-    at.mkdir(dir);
-    let env_cd = std::env::current_dir().unwrap();
-    if let Ok(result) = run_ucmd_as_root(&ts, &[dir, "--skip-chdir", "pwd"]) {
-        // Should return the same path
-        assert_eq!(
-            result.success().no_stderr().stdout_str(),
-            env_cd.to_str().unwrap()
-        );
-    } else {
-        print!("Test skipped; requires root user");
+    let at = ts.fixtures.clone();
+    let dirs = ["/", "/.", "/..", "isroot"];
+    at.symlink_file("/", "isroot");
+    for dir in dirs {
+        let env_cd = std::env::current_dir().unwrap();
+        if let Ok(result) = run_ucmd_as_root(&ts, &[dir, "--skip-chdir"]) {
+            // Should return the same path
+            assert_eq!(
+                result.success().no_stderr().stdout_str(),
+                env_cd.to_str().unwrap()
+            );
+        } else {
+            print!("Test skipped; requires root user");
+        }
     }
 }