owasp-modsecurity / ModSecurity

ModSecurity is an open source, cross platform web application firewall (WAF) engine for Apache, IIS and Nginx. It has a robust event-based programming language which provides protection from a range of attacks against web applications and allows for HTTP traffic monitoring, logging and real-time analysis.
https://www.modsecurity.org
Apache License 2.0
7.7k stars 1.54k forks source link

; incorrectly replaced by space in cmdline #3051

Closed marcstern closed 4 months ago

sonarcloud[bot] commented 4 months ago

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

dune73 commented 4 months ago

Hey @marcstern, what is this PR doing exactly?

marcstern commented 4 months ago

When using the t:cmdline transformation, the semicolon character was replaced by a space. An attack pattern like "xxx;sftp ..." would thus not be detected as the command "xxx" followed by the command "sftp", but only a string "xxx sftp ...", leading to a false negative. This PR fixes that problem.

dune73 commented 4 months ago

This is very good. Thanks.

In fact CRS is only making limited use of t:cmdLine because of this and similar problems that blur an attack.

I no longer remember all the details of said discussion. But let's see if we can get somebody to join the discussion here.

theseion commented 4 months ago

@lifeforms probably knows best, he was the one who started to replace t:cmdLine issues with a python script that generated regular expressions with shell evasion patterns. As @marcstern already pointed out in another discussion, matching with the script obviously works differently than t:cmdLine and will not necessarily produce the same results.

I can't give you any examples of issues that @lifeforms was trying to solve, but from my perspective, I think t:cmdLine tries to do too much and makes it hard to compose with other methods. Maybe it would be better to have multiple transformations that only do one thing, e.g., condense spaces, remove quotes, etc.. That could make it possible to use those transformations even in combination with @pmFromFile. Currently, if lines in the input to @pmFromFile are paths, t:cmdLine would remove all slashes and prevent matches.

I'm not taking performance into consideration, of course.

lifeforms commented 3 months ago

I've documented in 3.x why I started doing the custom preparation instead of using t:cmdLine:

For Unix:

# An effort was made to combat evasions by shell quoting (e.g. 'ls',
# 'l'"s", \l\s are all valid). ModSecurity has a t:cmdLine
# transformation built-in to deal with this, but unfortunately, it
# replaces ';' characters and lowercases the payload, which is less
# useful for this case. However, emulating the transformation makes
# the regexp more complex.

For Windows:

# An effort is made to combat evasions by CMD syntax; for example,
# the following strings are valid: c^md, @cmd, "c"md. ModSecurity
# has a t:cmdLine transformation built-in to deal with some of these,
# but unfortunately, that transformation replaces ';' characters (so
# we cannot match on the start of a command) and '\' characters (so we
# have trouble matching paths). This makes the regexp more complex.
dune73 commented 3 months ago

Thank you for the writeup @lifeforms. So it seems this PR fixes a long standing problem for CRS.

airween commented 3 months ago

Thanks @lifeforms.

Now we can modify this transformation in v3 too.