Rule 921110 #1883

azurit · 2020-09-13T12:45:40Z

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.

The text was updated successfully, but these errors were encountered:

azurit · 2020-09-13T17:43:57Z

Another realworld FP (editing WordPress CSS file):

/*
 * POST LIST
 */

airween · 2020-09-26T07:39:39Z

Hi @azurit,

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

azurit · 2020-09-26T09:21:05Z

@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 · 2020-09-28T07:23:10Z

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 · 2020-09-28T13:07:01Z

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.

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

What am I missing?

azurit · 2020-09-28T13:14:42Z

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

airween · 2020-09-28T13:16:56Z

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:

airween · 2020-09-28T13:47:08Z

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 · 2020-09-28T14:44:21Z

I was looking at the wrong version of the rule.

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

fgsch · 2020-09-28T15:11:39Z

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

airween · 2020-09-29T11:44:41Z

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 · 2020-10-20T20:26:38Z

@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 · 2020-10-20T20:50:14Z

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 · 2020-10-20T20:50:32Z

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 · 2020-10-20T20:54:16Z

- (?: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 · 2020-10-20T20:58:57Z

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 · 2020-10-20T21:08:08Z

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 · 2020-10-20T21:21:31Z

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

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)...

This drops pre-HTTP/1.0 support, partially reverting 80d2338. Should address coreruleset#1883.

lifeforms · 2021-01-04T20:32:10Z

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

azurit added the ➕ False Positive label Sep 13, 2020

azurit changed the title ~~Rule #921110~~ Rule 921110 Sep 13, 2020

dune73 mentioned this issue Sep 21, 2020

Monthly Chat Agenda September (2020-09-07) #1869

Closed

airween self-assigned this Sep 26, 2020

This was referenced Sep 29, 2020

phpBB 3.3.1 cannot submit data with common SQL terms. #1881

Closed

Wordpress FP fixes #1894

Merged

airween mentioned this issue Oct 5, 2020

Monthly Chat Agendas October (2020-10-05 and 2020-10-19) #1892

Closed

fgsch mentioned this issue Dec 22, 2020

Possible fix issue 1883 #1911

Closed

fgsch added a commit to fgsch/coreruleset that referenced this issue Jan 1, 2021

Rework pattern to fix false positive

3af9507

This drops pre-HTTP/1.0 support, partially reverting 80d2338. Should address coreruleset#1883.

fgsch mentioned this issue Jan 1, 2021

Rework pattern to fix false positive #1966

Merged

lifeforms closed this as completed Jan 4, 2021

jdevreese mentioned this issue Apr 13, 2021

False positive for rule 921110 #2054

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Rule 921110 #1883

Rule 921110 #1883

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Rule 921110 #1883

Rule 921110 #1883

Comments

Uh oh!

Audit Logs / Triggered Rule Numbers

Your Environment

Confirmation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!