Rule 921110

[azurit]

Audit Logs / Triggered Rule Numbers

FP can be easily triggered, for example, with this text in any POST field (realworld example from music forum):

DJ Wich - soundtrack Gympl
anything other here so previous line ends with EOL

On HTTP Request Smuggling webpage ( http://projects.webappsec.org/HTTP-Request-Smuggling ), example of exploitation is using two Content-Length headers. Is there any other way how to exploit this? If not, we should add something like this to the rule:
SecRule &REQUEST_HEADERS_NAMES:Content-Length "@ge 1"

EDIT: Seems explotation is possible also by other ways, see: https://www.cgisecurity.com/lib/HTTP-Request-Smuggling.pdf

Your Environment

• CRS version (e.g., v3.2.0): 3.3.0
• Paranoia level setting: PL1
• ModSecurity version (e.g., 2.9.3): 2.9.3
• Web Server and version (e.g., apache 2.4.41): 2.4.25
• Operating System and version: Debian Stretch

Confirmation

[x] I have removed any personal data (email addresses, IP addresses,
passwords, domain names) from any logs posted.

[azurit]

Another realworld FP (editing WordPress CSS file):

/*
 * POST LIST
 */

[airween]

Hi @azurit,

could you help us with a curl example? I'm tryinng to reproduce it but still can't.

[azurit]

@airween Using curl, i was able to reproduce it with HTML encoded EOL:

-d "message=DJ Wich soundtrack Gympl%0Danything other here so previous line ends with EOL"

[airween]

I've reviewed this rule and the issue. I'm wondering why is there the EOL characters at the end of used regex:

...(?:\s+http\/\d|[\r\n])

In the rule's comment I've found this:

This rule looks for a HTTP / WEBDAV method name in combination with the word http/\d or a CR/LF character.

But the given link I just found patterns only with string HTTP. I haven't been found any example for smuggling without HTTP/\d pattern.

These characters came into regex with this commit - @fgsch could you help us to explain what's the reason that we use EOL at there? I'm not sure that the pattern without HTTP/\d in any arguments should able to make a smuggling.

[fgsch]

That is in case the request is using pre-HTTP/1.0 (e.g., GET /\r\n).
I'm not sure I understand how this pattern is matching the input though.

Pattern: [\n\r]+(?:get|post|head|options|connect|put|delete|trace|track|patch|propfind|propatch|mkcol|copy|move|lock|unlock)\s+[^\s]+(?:\s+http|[\r\n])

Input:
DJ Wich - soundtrack Gympl%0Danything other here so previous line ends with EOL

What am I missing?

[azurit]

@fgsch Is there any web server which supports pre-HTTP/1.0?

[airween]

That is in case the request is using pre-HTTP/1.0 (e.g., GET /\r\n).

ah, thanks.

%0D will \r after transformations, so the relevant part of subject will be:

DJ Wich - soundtrack Gympl %0D anything other here so previous line ends with EOL

from the pattern:

(?:get|post|head|options|connect|put|delete|trace|track|patch|propfind|propatch|mkcol|copy|move|lock|unlock)\s+[^\s]+(?:\s+http|[\r\n])

[airween]

Here is the regex101 case with few remarks:

• first is this FP
• second is your case above (GET /r/n) - this isn't matched
• the others from the regression test cases, where the 5th case is negative test (marked)

[fgsch]

I was looking at the wrong version of the rule.

Actually, commit 8f53825 removed the leading [\r\n]+ that should have prevented this.

[fgsch]

@azurit Def some versions of apache and I believe nginx.

[airween]

A possible solution:

(?:get|post|head|options|connect|put|delete|trace|track|patch|propfind|propatch|mkcol|copy|move|lock|unlock)\s+(?:([\r\n]|\/\w|\/|http:\/\/))

Pattern and test cases are here.

Any remarks are welcome.

[ssigwart]

@airween, even with your update, multipart/form-data results in a lot of false positives. Some are below.

------WebKitFormBoundaryg15aUipbRYn3XubQ
Content-Disposition: form-data; name="test"

Clock
------WebKitFormBoundaryg15aUipbRYn3XubQ--

------WebKitFormBoundaryFtAB5BueWPz4uBaB
Content-Disposition: form-data; name="test"

Clock / chip time: 12:34
------WebKitFormBoundaryFtAB5BueWPz4uBaB--

What do you think of this rule? It removes the HTTP/0.9 version check, but maybe that can be put into a separate rule that can be enabled if HTTP/0.9 is enabled.

SecRule ARGS_NAMES|ARGS|REQUEST_BODY|XML:/* "@rx (?:get|post|head|options|connect|put|delete|trace|track|patch|propfind|propatch|mkcol|copy|move|lock|unlock)\s+(?:(\/\w|\/|http:\/\/))\w*[\r\n]" \

[ssigwart]

Actually, I forgot about the HTTP/1.1 part in this:

GET / HTTP/1.1
Host: www.example.com

Does this look right? I also added HTTPS checks.

- (?:get|post|head|options|connect|put|delete|trace|track|patch|propfind|propatch|mkcol|copy|move|lock|unlock)\s+(?:([\r\n]|\/\w|\/|http:\/\/))
+ (?:get|post|head|options|connect|put|delete|trace|track|patch|propfind|propatch|mkcol|copy|move|lock|unlock)\s+(?:(\/\w|\/|https?:\/\/))\S*\s+http\/

[airween]

Hi @ssigwart,

well, so many thanks for your update!

I don't see where is the matched sub-pattern in your first example. The LOCK from the Clock isn't, because there isn't any space and /, I guess.

The second example is good, because the lock / is matched.

I think the modified rule isn't good - just see the regex101.com link above, and replace the regex by your. You will see that the matches of most subjects will gone.

If you think that we should exclude the request with CT multipart/form-data, we should make an exception, or split the rule.

Or may be we should add some comment for the rule, how can user make the exception.

[airween]

- (?:get|post|head|options|connect|put|delete|trace|track|patch|propfind|propatch|mkcol|copy|move|lock|unlock)\s+(?:([\r\n]|\/\w|\/|http:\/\/))
+ (?:get|post|head|options|connect|put|delete|trace|track|patch|propfind|propatch|mkcol|copy|move|lock|unlock)\s+(?:(\/\w|\/|https?:\/\/))\S*\s+http\/

similar result with the modified pattern: there are few regression test case which doesn't match. Just try the regex101 link.

[ssigwart]

In the first example, that's because of the HTTP/0.9 check. I think \s+ matches the \r, then the [\r\n] matches the \n causing it to match on lock\r\n.

I tried the regex101 link on the second one. The regressions are get and get /foo. I think those are because I removed the rule to catch HTTP/0.9, but I feel like another rule geared towards them would be easier to construct and would allow people to remove the HTTP/0.9 rule easier if they don't support it.

If you think that we should exclude the request with CT multipart/form-data, we should make an exception, or split the rule.

I don't know enough about the vulnerability to make that call, but from my little knowledge, I think that would introduce an edge case that could be attacked.

Does this vulnerability always require http/X.X at the end to work on HTTP/1.0+? That's what I was banking on with the second attempt.

[airween]

Your first example matches with this modification:

------WebKitFormBoundaryg15aUipbRYn3XubQ
Content-Disposition: form-data; name="test"

Clock

------WebKitFormBoundaryg15aUipbRYn3XubQ--

as you can see, I had must to add a new line after the Clock.

I feel like another rule geared towards them would be easier to construct and would allow people to remove the HTTP/0.9 rule easier if they don't support it.

I don't want to break the original functionality of this rule. But may be we can't avoid that...

Does this vulnerability always require http/X.X at the end to work on HTTP/1.0+? That's what I was banking on with the second attempt.

That's a good question. I mention @fgsch now, hope he can helps us :)

[ssigwart]

as you can see, I had must to add a new line after the Clock.

For some reason, when using (?:get|post|head|options|connect|put|delete|trace|track|patch|propfind|propatch|mkcol|copy|move|lock|unlock)\s+(?:([\r\n]|\/\w|\/|http:\/\/)), I get it even without the blank line.

That's a good question. I mention @fgsch now, hope he can helps us :)

It would be great if the http/ did because searching for that should really limit false positives. Below is one from a PDF file upload, which would also be solved by that check.

...132 /Subtype /Widget /DA (/Helvetica 12 Tf 0 g)...

[lifeforms]

This should now be resolved by #1966. Thanks for reporting and helping to flesh out the issue!