Add path resolution for sudoedit (#1193)

Author: squell

src/common/context.rs

@@ -1,4 +1,4 @@
-use std::io;
+use std::{env, io};
 
 use crate::common::{HARDENED_ENUM_VALUE_0, HARDENED_ENUM_VALUE_1, HARDENED_ENUM_VALUE_2};
 use crate::exec::{RunOptions, Umask};
@@ -34,6 +34,9 @@ pub struct Context {
     pub use_pty: bool,
     pub noexec: bool,
     pub umask: Umask,
+    // sudoedit
+    #[cfg_attr(not(feature = "sudoedit"), allow(unused))]
+    pub files_to_edit: Vec<Option<SudoPath>>,
 }
 
 #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
@@ -74,7 +77,7 @@ impl Context {
             let path = if let Some(path) = override_path {
                 path
             } else {
-                system_path = std::env::var("PATH").unwrap_or_default();
+                system_path = env::var("PATH").unwrap_or_default();
                 system_path.as_ref()
             };
 
@@ -98,6 +101,7 @@ impl Context {
             use_pty: true,
             noexec: false,
             umask: Umask::Preserve,
+            files_to_edit: vec![],
         })
     }
 
@@ -109,12 +113,35 @@ impl Context {
         let (target_user, target_group) =
             resolve_target_user_and_group(&sudo_options.user, &sudo_options.group, &current_user)?;
 
+        // resolve file arguments; if something can't be resolved, don't add it to the "edit" list
+        let resolved_args = sudo_options.positional_args.iter().map(|arg| {
+            crate::common::resolve::canonicalize_newfile(arg)
+                .map_err(|_| arg)
+                .and_then(|path| path.into_os_string().into_string().map_err(|_| arg))
+        });
+
+        let files_to_edit = resolved_args
+            .clone()
+            .map(|path| path.ok().map(SudoPath::from_cli_string))
+            .collect();
+
+        // if a path resolved to something that isn't in UTF-8, it means it isn't in the sudoers file
+        // as well and so we treat it "as is" wrt. the policy lookup and fail if the user is allowed
+        // 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(),
+            })
+            .collect();
+
         // TODO: the more Rust way of doing things would be to create an alternative for sudoedit instead;
         // but a stringly typed interface feels the most decent thing to do (if we can pull it off)
-        // since "sudoedit" really is like a builtin command to sudo.
+        // since "sudoedit" really is like a builtin command to sudo. We may want to be a bit 'better' than
+        // ogsudo in the future.
         let command = CommandAndArguments {
             command: std::path::PathBuf::from("sudoedit"),
-            arguments: sudo_options.positional_args,
+            arguments,
             ..Default::default()
         };
 
@@ -135,6 +162,7 @@ impl Context {
             use_pty: true,
             noexec: false,
             umask: Umask::Preserve,
+            files_to_edit,
         })
     }
     pub fn from_validate_opts(sudo_options: SudoValidateOptions) -> Result<Context, Error> {
@@ -160,6 +188,7 @@ impl Context {
             use_pty: true,
             noexec: false,
             umask: Umask::Preserve,
+            files_to_edit: vec![],
         })
     }
 
@@ -182,7 +211,7 @@ impl Context {
             let path = if let Some(path) = override_path {
                 path
             } else {
-                system_path = std::env::var("PATH").unwrap_or_default();
+                system_path = env::var("PATH").unwrap_or_default();
                 system_path.as_ref()
             };
 
@@ -206,6 +235,7 @@ impl Context {
             use_pty: true,
             noexec: false,
             umask: Umask::Preserve,
+            files_to_edit: vec![],
         })
     }
 

src/common/resolve.rs

@@ -329,7 +329,20 @@ mod tests {
 /// Resolve symlinks in all the directories leading up to a file, but
 /// not the file itself; this allows sudo to specify a precise policy with
 /// tools like busybox or pgrep (which is a symlink to pgrep on systems)
+/// This function will check for existence.
 pub fn canonicalize<P: AsRef<Path>>(path: P) -> io::Result<PathBuf> {
+    let reconstructed_path = canonicalize_newfile(path)?;
+
+    // access the object to generate the regular error if it does not exist
+    let _ = fs::metadata(&reconstructed_path)?;
+
+    Ok(reconstructed_path)
+}
+
+/// Resolve symlinks in all the directories leading up to a file, but
+/// not the file itself; this allows us to keep symlinks as is, and will
+/// also work on non-existing files.
+pub fn canonicalize_newfile<P: AsRef<Path>>(path: P) -> io::Result<PathBuf> {
     let path = path.as_ref();
     let Some(parent) = path.parent() else {
         // path is "/" or a prefix
@@ -344,9 +357,6 @@ pub fn canonicalize<P: AsRef<Path>>(path: P) -> io::Result<PathBuf> {
         canon_path
     };
 
-    // access the object to generate the regular error if it does not exist
-    let _ = fs::metadata(&reconstructed_path)?;
-
     Ok(reconstructed_path)
 }
 
@@ -358,7 +368,7 @@ mod test {
     #[test]
     fn canonicalization() {
         assert_eq!(canonicalize("/").unwrap(), Path::new("/"));
-        assert_eq!(canonicalize("").unwrap(), Path::new(""));
+        assert!(canonicalize("").is_err());
         if cfg!(any(target_os = "linux", target_os = "macos")) {
             // this test REQUIRES /usr/bin/unxz to be a symlink for /usr/bin/xz
             assert_eq!(

src/sudo/env/tests.rs

@@ -136,6 +136,7 @@ fn create_test_context(sudo_options: SudoRunOptions) -> Context {
         noexec: false,
         bell: false,
         umask: Umask::Preserve,
+        files_to_edit: vec![],
     }
 }
 

src/sudo/pipeline/edit.rs

@@ -23,13 +23,19 @@ pub fn run_edit(edit_opts: SudoEditOptions) -> Result<(), Error> {
 
     let pid = context.process.pid;
 
+    for (path, arg) in context.files_to_edit.iter().zip(&context.command.arguments) {
+        if path.is_none() {
+            eprintln_ignore_io_error!("invalid path: {arg}")
+        }
+    }
+
     // run command and return corresponding exit code
     let command_exit_reason = {
         super::log_command_execution(&context);
 
         eprintln_ignore_io_error!(
             "this would launch sudoedit as requested, to edit the files: {:?}",
-            context.command.arguments.as_slice()
+            context.files_to_edit.as_slice()
         );
 
         Ok::<_, std::io::Error>(ExitReason::Code(42))