From 02f2684888115ec8a868f47998438dad480b91c7 Mon Sep 17 00:00:00 2001 From: Marc Schoolderman Date: Tue, 10 Jun 2025 12:20:33 +0200 Subject: [PATCH 1/4] add warning for FOLLOW tags --- src/sudoers/ast.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sudoers/ast.rs b/src/sudoers/ast.rs index f461683cd..4195812b7 100644 --- a/src/sudoers/ast.rs +++ b/src/sudoers/ast.rs @@ -395,7 +395,8 @@ impl Parse for MetaOrTag { "INTERCEPT is not supported by sudo-rs" ), // this is less fatal - "LOG_INPUT" | "NOLOG_INPUT" | "LOG_OUTPUT" | "NOLOG_OUTPUT" | "MAIL" | "NOMAIL" => { + "LOG_INPUT" | "NOLOG_INPUT" | "LOG_OUTPUT" | "NOLOG_OUTPUT" | "MAIL" | "NOMAIL" + | "FOLLOW" => { eprintln_ignore_io_error!( "warning: {} tags are ignored by sudo-rs", keyword.as_str() @@ -403,9 +404,8 @@ impl Parse for MetaOrTag { switch(|_| {})? } - // 'FOLLOW' and 'NOFOLLOW' are only usable in a sudoedit context, which will result in - // a parse error elsewhere. 'NOINTERCEPT' is the default behaviour. - "FOLLOW" | "NOFOLLOW" | "NOINTERCEPT" => switch(|_| {})?, + // 'NOFOLLOW' and 'NOINTERCEPT' are the default behaviour. + "NOFOLLOW" | "NOINTERCEPT" => switch(|_| {})?, "EXEC" => switch(|tag| tag.noexec = ExecControl::Exec)?, "NOEXEC" => switch(|tag| tag.noexec = ExecControl::Noexec)?, From d1942be1d22a7b9a47a580ff9c751490021f6ea8 Mon Sep 17 00:00:00 2001 From: Marc Schoolderman Date: Tue, 10 Jun 2025 15:46:53 +0200 Subject: [PATCH 2/4] add unit test --- src/sudoers/test/mod.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/sudoers/test/mod.rs b/src/sudoers/test/mod.rs index c0f01dcd9..a150460ff 100644 --- a/src/sudoers/test/mod.rs +++ b/src/sudoers/test/mod.rs @@ -374,6 +374,17 @@ fn inclusive_username() { assert_eq!(sirin, "şirin"); } +#[test] +fn sudoedit_recognized() { + let CommandSpec(_, Qualified::Allow(Meta::Only((cmd, args)))) = + parse_eval::("sudoedit /etc/tmux.conf") + else { + panic!(); + }; + assert_eq!(cmd.as_str(), "sudoedit"); + assert_eq!(args.unwrap().as_ref(), &["/etc/tmux.conf"][..]); +} + #[test] fn directive_test() { let y = parse_eval::>; From a62a153df709c87836c515cf97b7090e8d0bf3cc Mon Sep 17 00:00:00 2001 From: Marc Schoolderman Date: Tue, 10 Jun 2025 15:34:08 +0200 Subject: [PATCH 3/4] add sudoedit to the parser --- src/sudoers/ast.rs | 25 ------------------------- src/sudoers/tokens.rs | 19 ++++++++++++++++--- 2 files changed, 16 insertions(+), 28 deletions(-) diff --git a/src/sudoers/ast.rs b/src/sudoers/ast.rs index 4195812b7..4e7bd7489 100644 --- a/src/sudoers/ast.rs +++ b/src/sudoers/ast.rs @@ -471,31 +471,6 @@ impl Parse for CommandSpec { } } - let start_pos = stream.get_pos(); - if let Some(Username(keyword)) = try_nonterminal(stream)? { - if keyword == "sudoedit" { - // note: special behaviour of forward slashes in wildcards, tread carefully - unrecoverable!(pos = start_pos, stream, "sudoedit is not yet supported"); - } else if keyword == "list" { - return make(CommandSpec( - tags, - Allow(Meta::Only((glob::Pattern::new("list").unwrap(), None))), - )); - } else if keyword.starts_with("sha") { - unrecoverable!( - pos = start_pos, - stream, - "digest specifications are not supported" - ) - } else { - unrecoverable!( - pos = start_pos, - stream, - "expected command but found {keyword}" - ) - }; - } - let cmd: Spec = expect_nonterminal(stream)?; make(CommandSpec(tags, cmd)) diff --git a/src/sudoers/tokens.rs b/src/sudoers/tokens.rs index 749868e11..b9796f94c 100644 --- a/src/sudoers/tokens.rs +++ b/src/sudoers/tokens.rs @@ -192,10 +192,14 @@ impl Token for Command { Some(args.into_boxed_slice()) }; + if command.as_str() == "list" && argpat.is_some() { + return Err("list does not take arguments".to_string()); + } + Ok((command, argpat)) } - // all commands start with "/" except "sudoedit" + // all commands start with "/" except "sudoedit" or "list" fn accept_1st(c: char) -> bool { SimpleCommand::accept_1st(c) } @@ -218,6 +222,15 @@ impl Token for SimpleCommand { pat.map_err(|err| format!("wildcard pattern error {err}")) }; + // detect the two edges cases + if cmd == "list" || cmd == "sudoedit" { + return cvt_err(glob::Pattern::new(&cmd)); + } else if cmd.starts_with("sha") { + return Err("digest specifications are not supported".to_string()); + } else if !cmd.starts_with('/') { + return Err("fully qualified path needed".to_string()); + } + // record if the cmd ends in a slash and remove it if it does let is_dir = cmd.ends_with('/') && { cmd.pop(); @@ -240,9 +253,9 @@ impl Token for SimpleCommand { cvt_err(glob::Pattern::new(&cmd)) } - // all commands start with "/" except "sudoedit" + // all commands start with "/" except "sudoedit" or "list" fn accept_1st(c: char) -> bool { - c == '/' || c == 's' + c == '/' || c == 's' || c == 'l' } fn accept(c: char) -> bool { From 2bc316de00ad704bf0bf17faa8e47df2140700de Mon Sep 17 00:00:00 2001 From: Marc Schoolderman Date: Tue, 10 Jun 2025 16:22:00 +0200 Subject: [PATCH 4/4] missing unit tests related to 'list' pseudocommand --- src/sudoers/test/mod.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/sudoers/test/mod.rs b/src/sudoers/test/mod.rs index a150460ff..64e36b423 100644 --- a/src/sudoers/test/mod.rs +++ b/src/sudoers/test/mod.rs @@ -262,6 +262,10 @@ fn permission_test() { // apparmor #[cfg(feature = "apparmor")] pass!(["ALL ALL=(ALL:ALL) APPARMOR_PROFILE=unconfined ALL"], "user" => root(), "server"; "/bin/bar" => [apparmor_profile: Some("unconfined".to_string())]); + + // list + pass!(["ALL ALL=(ALL:ALL) /bin/ls, list"], "user" => root(), "server"; "list"); + FAIL!(["ALL ALL=(ALL:ALL) ALL, !list"], "user" => root(), "server"; "list"); } #[test] @@ -385,6 +389,12 @@ fn sudoedit_recognized() { assert_eq!(args.unwrap().as_ref(), &["/etc/tmux.conf"][..]); } +#[test] +#[should_panic = "list does not take arguments"] +fn list_does_not_take_args() { + parse_eval::("list /etc/tmux.conf"); +} + #[test] fn directive_test() { let y = parse_eval::>;