Skip to content
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.

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

Text/plain or custom Content-Type bypasses body evaluation. #438

Closed
M4tteoP opened this issue Sep 27, 2022 · 11 comments
Closed

Text/plain or custom Content-Type bypasses body evaluation. #438

M4tteoP opened this issue Sep 27, 2022 · 11 comments
Milestone

Comments

@M4tteoP
Copy link
Member

M4tteoP commented Sep 27, 2022

Description

POST requests with an explicit Content-Type header equal to text/plain or even just an empty space lead to not analyzing the body and directly sending it to the server.
I'm running a Coraza instance with a simple rule that checks the request body against a string (maliciouspayload):

...
SecRequestBodyAccess On
SecRule REQUEST_BODY "@rx maliciouspayload" "id:102,phase:2,t:lowercase,deny"
...

Curl requests like the followings, with explicit Content-Type different than application/x-www-form-urlencoded are leading to not checking the body:

  • curl -i -X POST -H 'Content-Type: text/plain' 'http://localhost:8090' --data "maliciouspayload"
  • curl -i -X POST -H 'Content-Type: ' 'http://localhost:8090' --data "maliciouspayload"

Logs:

[Evaluating phase 2
[[102] Evaluating rule 102
[[102] Expanding 1 arguments for rule 102
[[102] Transforming argument "" for rule 102
[[102] Arguments transformed for rule 102: []
[[102] Evaluating operator "@rx maliciouspayload" against "": NO MATCH
[[102] Finish evaluating rule 102
[Finished phase 2

"@rx maliciouspayload" against "": NO MATCH particularly caught my eye, meaning that the evaluation is performed, but against an empty string.

While, with application/x-www-form-urlencoded, the body is evaluated and the request is denied:

  • curl -i -X POST -H 'Content-Type: application/x-www-form-urlencoded' 'http://localhost:8090' --data "maliciouspayload"

Logs:

Attempting to process request body using "URLENCODED"
Evaluating phase 2
[102] Evaluating rule 102
[102] Expanding 1 arguments for rule 102
[102] Transforming argument "maliciouspayload" for rule 102
[102] Arguments transformed for rule 102: [maliciouspayload]
[102] Matching rule 102 REQUEST_BODY:
[102] Evaluating action log for rule 102
[102] Evaluating action auditlog for rule 102
[102] Evaluating action t for rule 102
[102] Evaluating operator "@rx maliciouspayload" against "maliciouspayload": MATCH
[102] Disrupting transaction by rule 102
[102] Evaluating action deny for rule 102
rule 102 matched

Steps to reproduce

Curl commands above against this branch (http-server example).

Is this an intended behaviour evaluating only super specific Content-Types? If so, shouldn't this behaviour be handled by the CRS rule 900220 that checks allowed Content-Types?

Thanks!

@jptosso
Copy link
Member

jptosso commented Sep 27, 2022

Hey! Evaluation of content-types for urlencoded is only enforced if content-type is x-www-form-urlencoded or the CTL variable ForceRequestBodyVariable is set. Rule 900220 should stop an unknown content-type. I will look into it

@M4tteoP
Copy link
Member Author

M4tteoP commented Sep 27, 2022

Hi @jptosso!
Just to specify:

  • setting application/x-www-form-urlencoded seems to actually be the only way to parse the body request.
  • Speaking about 900220, I was just thinking loud about CRS duties and Coraza ones, in the example, the CRS is NOT loaded.

I debugged a bit and seems that if no processing is done on the body, the latter is not added to the transaction. With #439 it is now working. Could it make sense to have a generic bodyprocessor that just performs this option of populating the transaction and will performed with no other bodyprocessors are triggered Inside the (if rbp == "" {)?

Thanks for the help and feedbacks

@jptosso
Copy link
Member

jptosso commented Sep 27, 2022

There is a design reason for that. There are many cases where we just want to avoid processing the body, mostly because of resources and DDOS. Specific pages, specific mimes, specific lengths.

We cannot accept all bodies by default, it must be custom tailored by the user according to his needs. For that exact reason ForceRequestBodyVariable exists, if you want to force the request body you may use it.

@M4tteoP
Copy link
Member Author

M4tteoP commented Sep 27, 2022

I see, and I understand the reasoning behind the necessity of defining boundaries between analyzing it or not.
I'm just not so sure about the overall default behaviour.
By default:

  • A request with a payload as simple as text/plain is not evaluated, but always allowed.
  • A request with a payload with content typeapplication/x-www-form-urlencoded is processed and (always?) evaluated.
  • A request with any malformed content type is not evaluated, but always allowed.

Why should a user expect that a legit content type like text/plain is not being evaluated but application/x-www-form-urlencoded is? Currently bypassing a request-body rule is a matter of changing the content type to any value different than application/x-www-form-urlencoded. My mind is now getting trapped in a rabbit hole thinking that the only way to have this rule effective is indeed to enable ForceRequestBodyVariable. 😵‍💫

@jptosso
Copy link
Member

jptosso commented Sep 27, 2022

You can also force the body processor by ctl: requestBodyProcessor=JSON, for example.

If you would like to expand this discussion, we could check it on tomorrow's meeting #388

@jcchavezs
Copy link
Member

jcchavezs commented Oct 11, 2022 via email

@M4tteoP
Copy link
Member Author

M4tteoP commented Oct 12, 2022

Writing down a recap about when the request body is evaluated:

  1. First of all RequestBodyAccess requires to be On.
  2. By default, with just 1. the REQUEST_BODY variable is populated only if the content-type is application/x-www-form-urlencoded. Against any other content-type the variable will be empty and rules will never match.
  3. We can force to populate the REQUEST_BODY:
    • via ForceRequestBodyVariable
    • Enforcing a specific body processor. E.g. ctl:requestBodyProcessor=JSON

Speaking about running the CRS:

  • 3. is basically enforced during the initialization by rule 901340.
  • Content-type manipulation is mitigated by 900220 where it is possible to explicit the list of allowed content types.

To the best of my knowledge, ModSecurity:

  • v2: Behaves just like described above (Manual ref). (Not personally tested)
  • v3: Always populates the REQUEST_BODY therefore by default, regardless of the content-type, every request body is analyzed (ref). (tested)

If we are on the same page about the described behaviour I would close this issue and propose to just merging @jcchavezs's PR #441.

@jcchavezs jcchavezs added this to the v3 alpha milestone Nov 6, 2022
@jcchavezs jcchavezs added the v3 label Nov 6, 2022
@github-actions
Copy link

github-actions bot commented Dec 7, 2022

This issue is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale label Dec 7, 2022
@github-actions
Copy link

This issue was closed because it has been inactive for 14 days since being marked as stale.

@jcchavezs
Copy link
Member

Is this still an issue @M4tteoP?

@M4tteoP
Copy link
Member Author

M4tteoP commented Feb 1, 2023

I think that we somewhat agreed that the behaviour described in the previous comment) is the intended one. So, unless new considerations are made, we can keep this issue closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants