fix(920180): wrong unit test - content-type evasion bypass #4339
Conversation
|
📊 Quantitative test results for language: |
|
Very interesting find! |
|
Regarging the tests... I think go-ftw won't actually use http2 as a client and just use http/1.1 with the |
|
here's what the HTTP RFCs say about using PATCH, POST, or PUT methods without a Content-Type header: RFC 9110 / RFC 7231 (General HTTP Semantics)The HTTP specification IETF IETF states that a sender that generates a message containing content (payload body) SHOULD generate a Content-Type header field in that message unless the intended media type of the enclosed representation is unknown to the sender. If a Content-Type header field is not present, the recipient MAY either assume a media type of "application/octet-stream" or examine the data to determine its type. IETFIETF Key points:
RFC 5789 (PATCH Method)For the PATCH method specifically, the set of changes is represented in a format called a "patch document" identified by a media type. Practical Implications
Best practice: Always include the Content-Type header for POST, PUT, and PATCH requests that contain a body, even though it's not strictly required by the specification. |
|
I assume this opens also a pandora's box regarding the default accepted types for PATCH. |
tests/regression/tests/REQUEST-920-PROTOCOL-ENFORCEMENT/920180.yaml
Outdated
Show resolved
Hide resolved
|
The rule targets requests < HTTP/2, which is why your payloads don't match. That being said, you're right, both the documentation and the test are wrong. We haven't yet implemented HTTP/2 in go-ftw, so we can't test anything with a protocol version other than HTTP/1.1. If we start blocking that behaviour, we need to make it configurable through |
|
Added coreruleset/go-ftw#556 as follow-up to see what we can do. |
….yaml Co-authored-by: Max Leske <[email protected]>
3 participants
Hello,
I think this unit test is misleading - in real-word context, rule 920180 is not triggered by this payload :
curl -v --http2 -H "x-format-output: txt-matched-rules" -H "x-crs-paranoia-level:4" "http://sandbox.coreruleset.org/" -d '{"id_order":"select(sleep(10));"}' -H 'Content-Type:' -H 'Content-Length:'920180 PL1 POST without Content-Length or Transfer-Encoding headers
This is a content-type evasion bypass to avoid these rules :
curl -v --http2 -H "x-format-output: txt-matched-rules" -H "x-crs-paranoia-level:4" "http://sandbox.coreruleset.org/" -d '{"id_order":"select(sleep(10));"}' -H 'Content-Type:application/json'920273 PL4 Invalid character in request (outside of very strict set)
933161 PL3 PHP Injection Attack: Low-Value PHP Function Call Found
942100 PL1 SQL Injection Attack Detected via libinjection
942160 PL1 Detects blind sqli tests using sleep() or benchmark()
942150 PL2 SQL Injection Attack: SQL function name detected
942410 PL2 SQL Injection Attack
942432 PL4 Restricted SQL Character Anomaly Detection (args): # of special characters exceeded (2)
This payload targets backends that use file_get_contents('php://input');, which is common in AJAX transactions (especially in the PrestaShop ecosystem).
PR available here for mitigation : #4341
I don’t think this rule (920180) is actually useful in its current state.
If you agree, I’d suggest a rewrite - among the rules we have here regarding evasions, we completely forbid the use of PATCH, POST, or PUT methods being sent without a Content-Type header.
What do you think ?