Skip to content

Conversation

@squell
Copy link
Member

@squell squell commented Nov 25, 2025

Right now to only give a more precise error message, CC #1350

@squell squell added the C-parser Parser/AST label Nov 25, 2025
@bjorn3
Copy link
Collaborator

bjorn3 commented Nov 25, 2025

The regex is still passed through glob::Pattern::new() which can error on valid regexes like ^a**$ I think.

@squell
Copy link
Member Author

squell commented Nov 25, 2025

The regex is still passed through glob::Pattern::new() which can error on valid regexes like ^a**$ I think.

I only considered the case where the program arguments are a regular expression. Since the example given on the sudo blogpost only deals with that (and only the wildcards-in-arguments feature is problematic), I had missed that in sudoers the command can also be a regular expression (🙈); to quote man sudoers:

In sudoers, regular expressions must start with a ‘^’ character and end
with a ‘$’. This makes it explicit what is, or is not, a regular
expression. **Either the path name, the command line arguments or both
may be regular expressions. ** Because the path name and arguments are
matched separately
, it is even possible to use wildcards for the path
name and regular expressions for the arguments. It is not possible to
use a single regular expression to match both the command and its
arguments
. Regular expressions in sudoers are limited to 1024
characters.

I'll add a seperate test-case and patch for that.

Note: the current code already requires that a command always starts with /, so a command being specified as a regular expression already gives an error:

/etc/sudoers:42:12: expected path to binary (or sudoedit)
squell ALL=^/bin/ls$ -la
           ^

But I've changed that to also complain about regular expressions.

Note that in ogsudo, the following is not a correct way (per the quoted manpage above, and you can also verify this) to write a regex match for command-and-arguments:

squell ALL=^/bin/ls -la$

But it wil not give any error message to that effect (it will try to match a binary with the name ls -la, which you can run with whatever argument you like)

@squell squell force-pushed the regex-error branch 2 times, most recently from 031bbdd to d437fdc Compare November 25, 2025 18:27
@bjorn3 bjorn3 merged commit f68e342 into main Nov 26, 2025
17 checks passed
@bjorn3 bjorn3 deleted the regex-error branch November 26, 2025 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-parser Parser/AST

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants