expand alias table while printing in --list instead of unfolding in memory; fixes #979

Author: squell

src/sudoers/ast.rs

@@ -25,13 +25,6 @@ impl<T> Qualified<T> {
         }
     }
 
-    pub fn negate(&self) -> Qualified<&T> {
-        match self {
-            Qualified::Allow(item) => Qualified::Forbid(item),
-            Qualified::Forbid(item) => Qualified::Allow(item),
-        }
-    }
-
     #[cfg(test)]
     pub fn as_allow(&self) -> Option<&T> {
         if let Self::Allow(v) = self {

src/sudoers/entry.rs

@@ -12,25 +12,32 @@ use crate::{
 use self::verbose::Verbose;
 
 use super::{
-    ast::{Authenticate, RunAs, Tag},
+    ast::{Authenticate, Def, RunAs, Tag},
     tokens::Command,
+    VecOrd,
 };
 
 mod verbose;
 
 pub struct Entry<'a> {
     run_as: Option<&'a RunAs>,
     cmd_specs: Vec<(Tag, Qualified<&'a Meta<Command>>)>,
+    cmd_alias: &'a VecOrd<Def<Command>>,
 }
 
 impl<'a> Entry<'a> {
     pub(super) fn new(
         run_as: Option<&'a RunAs>,
         cmd_specs: Vec<(Tag, Qualified<&'a Meta<Command>>)>,
+        cmd_alias: &'a VecOrd<Def<Command>>,
     ) -> Self {
         debug_assert!(!cmd_specs.is_empty());
 
-        Self { run_as, cmd_specs }
+        Self {
+            run_as,
+            cmd_specs,
+            cmd_alias,
+        }
     }
 
     pub fn verbose(self) -> impl fmt::Display + 'a {
@@ -56,7 +63,11 @@ fn root_runas() -> RunAs {
 
 impl fmt::Display for Entry<'_> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let Self { run_as, cmd_specs } = self;
+        let Self {
+            run_as,
+            cmd_specs,
+            cmd_alias,
+        } = self;
 
         let root_runas = root_runas();
         let run_as = run_as.unwrap_or(&root_runas);
@@ -79,7 +90,7 @@ impl fmt::Display for Entry<'_> {
 
             write_tag(f, tag, last_tag)?;
             last_tag = Some(tag);
-            write_spec(f, spec)?;
+            write_spec(f, spec, cmd_alias, true)?;
         }
 
         Ok(())
@@ -208,17 +219,28 @@ fn write_tag(f: &mut fmt::Formatter, tag: &Tag, last_tag: Option<&Tag>) -> fmt::
     Ok(())
 }
 
-fn write_spec(f: &mut fmt::Formatter, spec: &Qualified<&Meta<Command>>) -> fmt::Result {
+fn write_spec(
+    f: &mut fmt::Formatter,
+    spec: &Qualified<&Meta<Command>>,
+    alias_list: &VecOrd<Def<Command>>,
+    mut sign: bool,
+) -> fmt::Result {
     let meta = match spec {
         Qualified::Allow(meta) => meta,
         Qualified::Forbid(meta) => {
-            f.write_str("!")?;
+            sign = !sign;
             meta
         }
     };
 
+    match meta {
+        Meta::All | Meta::Only(_) if !sign => f.write_str("!")?,
+        _ => {}
+    }
+
     match meta {
         Meta::All => f.write_str("ALL")?,
+
         Meta::Only((cmd, args)) => {
             write!(f, "{cmd}")?;
             if let Some(args) = args {
@@ -227,7 +249,22 @@ fn write_spec(f: &mut fmt::Formatter, spec: &Qualified<&Meta<Command>>) -> fmt::
                 }
             }
         }
-        Meta::Alias(alias) => f.write_str(alias)?,
+        Meta::Alias(alias) => {
+            // this will terminate, since AliasTable has been checked by sanitize_alias_table
+            if let Some(Def(_, spec_list)) = super::elems(alias_list).find(|Def(id, _)| id == alias)
+            {
+                let mut is_first_iteration = true;
+                for spec in spec_list {
+                    if !is_first_iteration {
+                        f.write_str(", ")?;
+                    }
+                    write_spec(f, &spec.as_ref(), alias_list, sign)?;
+                    is_first_iteration = false;
+                }
+            } else {
+                f.write_str("???")?
+            }
+        }
     }
 
     Ok(())

src/sudoers/entry/verbose.rs

@@ -11,7 +11,11 @@ pub struct Verbose<'a>(pub Entry<'a>);
 
 impl fmt::Display for Verbose<'_> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let Self(Entry { run_as, cmd_specs }) = self;
+        let Self(Entry {
+            run_as,
+            cmd_specs,
+            cmd_alias,
+        }) = self;
 
         let root_runas = super::root_runas();
         let run_as = run_as.unwrap_or(&root_runas);
@@ -31,7 +35,7 @@ impl fmt::Display for Verbose<'_> {
             last_tag = Some(tag);
 
             f.write_str("\n\t")?;
-            super::write_spec(f, cmd_spec)?;
+            super::write_spec(f, cmd_spec, cmd_alias, true)?;
         }
 
         Ok(())

src/sudoers/mod.rs

@@ -179,10 +179,9 @@ impl Sudoers {
         // ensure `sudo $command` and `sudo --list` use the same permission checking logic
         let user_specs = self.matching_user_specs(invoking_user, hostname);
 
-        let cmnd_aliases = unfold_alias_table(&self.aliases.cmnd);
         let mut entries = vec![];
         for cmd_specs in user_specs {
-            group_cmd_specs_per_runas(cmd_specs, &mut entries, &cmnd_aliases);
+            group_cmd_specs_per_runas(cmd_specs, &mut entries, &self.aliases.cmnd);
         }
 
         entries
@@ -214,41 +213,29 @@ impl Sudoers {
 fn group_cmd_specs_per_runas<'a>(
     cmnd_specs: impl Iterator<Item = (Option<&'a RunAs>, (Tag, &'a Spec<Command>))>,
     entries: &mut Vec<Entry<'a>>,
-    cmnd_aliases: &HashMap<&String, &'a Vec<Spec<Command>>>,
+    cmnd_aliases: &'a VecOrd<Def<Command>>,
 ) {
     let mut last_runas = None;
     let mut collected_specs = vec![];
 
     for (runas, (tag, spec)) in cmnd_specs {
         if runas.map(|r| r as *const _) != last_runas.map(|r| r as *const _) {
             if !collected_specs.is_empty() {
-                entries.push(Entry::new(last_runas, mem::take(&mut collected_specs)));
+                entries.push(Entry::new(
+                    last_runas,
+                    mem::take(&mut collected_specs),
+                    cmnd_aliases,
+                ));
             }
 
             last_runas = runas;
         }
 
-        let (negate, meta) = match spec {
-            Qualified::Allow(meta) => (false, meta),
-            Qualified::Forbid(meta) => (true, meta),
-        };
-
-        if let Meta::Alias(alias_name) = meta {
-            if let Some(specs) = cmnd_aliases.get(alias_name) {
-                // expand Cmnd_Alias
-                for spec in specs.iter() {
-                    let new_spec = if negate { spec.negate() } else { spec.as_ref() };
-
-                    collected_specs.push((tag.clone(), new_spec))
-                }
-            }
-        } else {
-            collected_specs.push((tag, spec.as_ref()));
-        }
+        collected_specs.push((tag, spec.as_ref()));
     }
 
     if !collected_specs.is_empty() {
-        entries.push(Entry::new(last_runas, collected_specs));
+        entries.push(Entry::new(last_runas, collected_specs, cmnd_aliases));
     }
 }
 
@@ -481,10 +468,6 @@ fn match_command<'a>((cmd, args): (&'a Path, &'a [String])) -> (impl Fn(&Command
     }
 }
 
-fn unfold_alias_table<T>(table: &VecOrd<Def<T>>) -> HashMap<&String, &Vec<Qualified<Meta<T>>>> {
-    elems(table).map(|Def(id, list)| (id, list)).collect()
-}
-
 /// Find all the aliases that a object is a member of; this requires [sanitize_alias_table] to have run first;
 /// I.e. this function should not be "pub".
 fn get_aliases<Predicate, T>(table: &VecOrd<Def<T>>, pred: &Predicate) -> FoundAliases