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
8.31k stars 1.61k forks source link

SecRequestBodyAccess off skips the phase:2 rules #2465

Open airween opened 3 years ago

airween commented 3 years ago

Describe the bug

If the SecRequestBodyAccess is off, then the engine skips all rules which has phase:2 action.

To Reproduce

Here you can find a regression test case.

In the debug log you can see this line:

[nnnnnn] [/?q=evil] [4] Request body processing is disabled

Expected behavior

As you can see, the request contains an XML payload (CT doesn't count...), and only one (relevant) rule with id:1. The rule has two variable: REQUEST_URI and REQUEST_BODY. First one available in phase 1, but the second one only in phase 2, so the rule set to phase 2. The request was constructed that both variable matches, but the test returns with 200.

The expected behavior is 403, because the config disables only the processing of request body, not the whole phase 2.

If you replace the phase:2 by phase:1 in rule 1, then the request will blocked as it expected.

Server (please complete the following information):

Additional context

There are two another issues related to this wrong behavior: #1940 - CRS issue - this helped me to describe this behavior

2273 - ModSecurity - SecRequestBodyAccess Off doesn't fully disable request body processing

I checked the source, and found a very strange solution to handle the SecRequestBodyAccess - looks like the appendBody() allocates a buffer even thought the variable set it to off. In addition, the chosen body processor also woke up, and the payload will parsed. Checking the body access is comes only at this point, that's why the #2273 exists. I assume that users disable the body access to save CPU/memory (as reporter also wrote there), but in this isn't fulfilled.

The another strange behavior is if the SecRequestBodyAccess is off, then whole phase 2 will ignored (in line 895-911), because the evaluation called later, in L938.

Note: I didn't checked, but I suppose the ctl:requestBodyAccess=off would give same result.

defanator commented 3 years ago

@airween yes please - go ahead then! Thanks.

defanator commented 3 years ago

Backtrace from coredump obtained after running nginx-debug -t:

JFTR, this has been fixed with the following change (thanks @zimmerle): https://github.com/SpiderLabs/ModSecurity/commit/50fc347ed4d0852d54561e15559de07c698fbb16

(I see it hasn't been arrived to v3/master yet but hopefully it will be there soon.)

dune73 commented 3 years ago

@defanator : Could be very clear what exactly 50fc347 fixes? There are quite a few deviations being discussed here and I'd hate to run into misunderstandings.

zimmerle commented 3 years ago

JFTR, this has been fixed with the following change (thanks @zimmerle): 50fc347

(I see it hasn't been arrived to v3/master yet but hopefully it will be there soon.)

Now on v3/master :rocket: :rocket:

it was under tests at Workflow: Quality Assurance - 50fc347, QA is happy about it :satisfied::satisfied::satisfied::satisfied:

defanator commented 3 years ago

@defanator : Could be very clear what exactly 50fc347 fixes? There are quite a few deviations being discussed here and I'd hate to run into misunderstandings.

It fixes segfault on starting up nginx with debug version of the ModSecurity-nginx connector module and loaded CRS.

dune73 commented 3 years ago

We have published a blog post covering this vulnerability and what users can do about it while there is no official fix.

https://coreruleset.org/20210302/disabling-request-body-access-in-modsecurity-3-leads-to-complete-bypass/

Sorry this took so long, but we ran into several issues while testing our workaround. These issues also include another ModSecurity3 bug (see above).