8000 fix(security): alias false negative by Xhoenix · Pull Request #3740 · coreruleset/coreruleset · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix(security): alias false negative #3740

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 14 commits into from
Jun 28, 2024
Merged

fix(security): alias false negative #3740

merged 14 commits into from
Jun 28, 2024

Conversation

Xhoenix
Copy link
Member
@Xhoenix Xhoenix commented Jun 20, 2024 •
edited
Loading

Unix alias bypass with -p flag in bash shell.

fzipi
fzipi reviewed Jun 20, 2024
View reviewed changes
@Xhoenix @fzipi
Update tests/regression/tests/REQUEST-932-APPLICATION-ATTACK-RCE/9321…
d2485e7 
…75.yaml

Co-authored-by: Felipe Zipitría <3012076+fzipi@users.noreply.github.com>
fzipi
fzipi reviewed Jun 20, 2024
View reviewed changes
Copy link
Member
@fzipi fzipi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I read, using -p just prints the aliases. How impactful would be that?

@Xhoenix
Copy link
Member Author
Xhoenix commented Jun 20, 2024

It prints and also creates an alias in the same command. For e.g,

alias -p testxfr=id

works the same as:

alias testxfr=id

@fzipi
Copy link
Member
fzipi commented Jun 20, 2024

Looks like zsh has lots of additional options:

       alias [ {+|-}gmrsL ] [ name[=value] ... ]
              For each name with a corresponding value, define an alias with that value.  A trailing space in value causes the next word to
              be checked for alias expansion.  If the -g flag is present, define a global alias; global aliases are expanded even if they
              do not occur in command position:

                     % perldoc --help 2>&1 | grep 'built-in functions'
                         -f   Search Perl built-in functions
                     % alias -g HG='--help 2>&1 | grep'
                     % perldoc HG 'built-in functions'
                         -f   Search Perl built-in functions

              If the -s flag is present, define a suffix alias: if the command word on a command line is in the form `text.name', where
              text is any non-empty string, it is replaced by the text `value text.name'.  Note that name is treated as a literal string,
              not a pattern.  A trailing space in value is not special in this case.  For example,

                     alias -s ps='gv --'

              will cause the command `*.ps' to be expanded to `gv -- *.ps'.  As alias expansion is carried out earlier than globbing, the
              `*.ps' will then be expanded.  Suffix aliases constitute a different name space from other aliases (so in the above example
              it is still possible to create an alias for the command ps) and the two sets are never listed together.

              For each name with no value, print the value of name, if any.  With no arguments, print all currently defined aliases other
              than suffix aliases.  If the -m flag is given the arguments are taken as patterns (they should be quoted to preserve them
              from being interpreted as glob patterns), and the aliases matching these patterns are printed.  When printing aliases and one
              of the -g, -r or -s flags is present, restrict the printing to global, regular or suffix aliases, respectively; a regular
              alias is one which is neither a global nor a suffix alias.   Using `+' instead of `-', or ending the option list with a
              single `+', prevents the values of the aliases from being printed.

              If the -L flag is present, then print each alias in a manner suitable for putting in a startup script.  The exit status is
              nonzero if a name (with no value) is given for which no alias has been defined.

              For more on aliases, include common problems, see the section ALIASING in zshmisc(1).

@Xhoenix
Copy link
Member Author
Xhoenix commented Jun 20, 2024

Yup, looks like zsh also does. I only checked bash because it's the most commonly used shell in servers, but looks like now we've to add zsh too, which would mean we probably need to take into consideration other unix shells. What do you suggest we should do:

  1. Add the zsh flags
  2. Scrap this PR and accept the bypass. (:
  3. Or some other approach?

@Xhoenix
Copy link
Member Author
Xhoenix commented Jun 20, 2024

I think a better approach would be to use a single regex pattern to catch all the flags, like:-

\-[a-zA-Z]

@fzipi
Copy link
Member
fzipi commented Jun 20, 2024

Sure, that will work. Check first if the regex has insensitive comparison before adding A-Z.

@azurit
Copy link
Member
azurit commented Jun 22, 2024

@Xhoenix You are missing (?:...) in the regex so now it matches any equation like a=b, no alias keyword is needed.

@azurit
Copy link
Member
azurit commented Jun 22, 2024 •
edited
Loading

Also, these needs to be handled if we want to support zsh (all of them should be valid, according to docs):

alias -L a=b
alias -gmr a=b
alias +g a=b
alias +m+ a=b

Additionaly, some examples from the docs not matched right now:

alias ls-al='ls -al'
alias cd..='cd ..'
alias ..='cd ..'
alias .='echo $PWD'

@azurit
Copy link
Member
azurit commented Jun 22, 2024 •
edited
Loading

After another look, i think you should completely change your regex. Your current work is:
(?:\-[a-z]\b\s+['"\w!%,@]+|['"\w!%,@]+)

i.e. you copied original regex ['"\w!%,@]+ and extended it. This isn't good because now we need to maintain two copies of ['"\w!%,@]+ which are supposed to match the same thing. Instead, it's better to use ? to mark that part of the regex that matches arguments may appear zero or one times:
(?:-[a-z]\b\s+)?['"\w!%,@]+

@Xhoenix
Copy link
Member Author
Xhoenix commented Jun 22, 2024 •
edited
Loading

I think that do need to be improved. According to our documentation , using an or(|) in the assembly file will create the necessary regex you mentioned.

@azurit
Copy link
Member
azurit commented Jun 22, 2024 •
edited
Loading

According to our documentation, using an or(|) in the assembly file will create the necessary regex you mentioned.

Can you send a link? It's definitely not created. I recommend to test your regexes for example on https://regex101.com/ .

azurit
azurit reviewed Jun 23, 2024
View reviewed changes
@azurit
Copy link
Member
azurit commented Jun 23, 2024

Can you resolve also these?

@Xhoenix @azurit
Update regex-assembly/932175.ra
399bf7b 
Co-authored-by: azurit <jozef@sudolsky.sk>
@Xhoenix
Copy link
Member Author
Xhoenix commented Jun 23, 2024

I checked and the alias -L doesn't works with a payload. We need to take care of the rest.

@Xhoenix Xhoenix requested a review from theseion June 26, 2024 07:16
azurit
azurit reviewed Jun 26, 2024
View reviewed changes
azurit
azurit reviewed Jun 26, 2024
View reviewed changes
Xhoenix and others added 2 commits June 26, 2024 17:59
@Xhoenix @azurit
Update regex-assembly/932175.ra
56d7bd0 
Co-authored-by: azurit <jozef@sudolsky.sk>
@Xhoenix @azurit
Update regex-assembly/932175.ra
3936260 
Co-authored-by: azurit <jozef@sudolsky.sk>
theseion
theseion requested changes Jun 27, 2024
View reviewed changes
67E6
@azurit
Copy link
Member
azurit commented Jun 28, 2024

@Xhoenix Feel free to merge this.

@Xhoenix Xhoenix added this pull request to the merge queue Jun 28, 2024
Merged via the queue into coreruleset:main with commit 88a551d Jun 28, 2024
4 checks passed
@Xhoenix Xhoenix changed the title fix: alias false negative fix(security): alias false negative Jun 29, 2024
@fzipi fzipi mentioned this pull request Jul 1, 2024
Closed
@fzipi fzipi mentioned this pull request Jul 23, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@azurit azurit azurit left review comments

@fzipi fzipi fzipi left review comments

@theseion theseion theseion approved these changes

Assignees
No one assigned
Labels
➖ False Negative - Evasion release:fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Xhoenix @fzipi @azurit @theseion
0