ModSecurity: REQUEST_BODY is set when it should not according to documentation

I think there is a mismatch between modsec-3 implementation and modsec documentation.

According to https://github.com/SpiderLabs/ModSecurity/wiki/Reference-Manual-(v2.x)#request_body the REQUEST_BODY variable should be set only under 2 conditions.

If the WWWFormURLEncoded body processor is used
If no body processor is used and ctl:forceRequestBodyVariable is used in phase:1

In current v3 code, this variables is always set.

It is also expected to be set in regression tests, for example test/test-cases/regression/variable-REQUEST_BODY.json. This test is using the Multipart processor (set from C code and not from modsec language)

However, the regression tests of the OWASP Modsec CoreRuleset team (https://github.com/SpiderLabs/owasp-modsecurity-crs/tree/v3.2/dev/util/regression-tests/tests) expect that REQUEST_BODY is not set in some cases. For example in regression test https://github.com/SpiderLabs/owasp-modsecurity-crs/blob/v3.2/dev/util/regression-tests/tests/REQUEST-944-APPLICATION-ATTACK-JAVA/944120.yaml#L128 against the rule

# Magic bytes detected and payload included possibly RCE vulnerable classess detected and process execution methods detected
# anomaly score set to critical as all conditions indicate the request try to perform RCE.
SecRule ARGS|ARGS_NAMES|REQUEST_COOKIES|!REQUEST_COOKIES:/__utm/|REQUEST_COOKIES_NAMES|REQUEST_BODY|REQUEST_HEADERS|XML:/*|XML://@* \
    "@rx (?:clonetransformer|forclosure|instantiatefactory|instantiatetransformer|invokertransformer|prototypeclonefactory|prototypeserializationfactory|whileclosure|getproperty|filewriter|xmldecoder)" \
    "id:944120,\
    phase:2,\
    block,\
    t:none,t:lowercase,\
    log,\
    msg:'Remote Command Execution: Java serialization (CVE-2015-5842)',\
    logdata:'Matched Data: %{MATCHED_VAR} found within %{MATCHED_VAR_NAME}',\
    tag:'application-multi',\
    tag:'language-java',\
    tag:'platform-multi',\
    tag:'attack-rce',\
    tag:'OWASP_CRS/WEB_ATTACK/COMMAND_INJECTION',\
    tag:'WASCTC/WASC-31',\
    tag:'OWASP_TOP_10/A1',\
    tag:'PCI/6.5.2',\
    tag:'paranoia-level/1',\
    ver:'OWASP_CRS/3.1.0',\
    severity:'CRITICAL',\
    chain"
    SecRule MATCHED_VARS "@rx (?:runtime|processbuilder)" \
        "t:none,t:lowercase,\
        setvar:'tx.msg=%{rule.msg}',\
        setvar:'tx.rce_score=+%{tx.critical_anomaly_score}',\
        setvar:'tx.anomaly_score_pl1=+%{tx.critical_anomaly_score}',\
        setvar:'tx.%{rule.id}-OWASP_CRS/WEB_ATTACK/RCE-%{MATCHED_VAR_NAME}=%{MATCHED_VAR}'"

the test is expecting that the rule is not matching. Which will only happen if REQUEST_BODY is not set.

So I can either make the Modsec 3 regression tests happy or the Modsec-CoreRuleset regression tests.

Is there a plan to change the behaviour or the implementation?

Thanks

About this issue

Original URL
State: open
Created 5 years ago
Comments: 16 (15 by maintainers)

Most upvoted comments

@zimmerle (by the way: @mirkodziadzka-avi here, not @michaelgranzow-avi 😃 Thanks for the explanation. That REQUEST_BODY is only set when needed is an implementation detail which does not impact the semantic. And it does make sense to avoid unneeded settings.

My question is about changing the semantic:

If you are looking for CoreRuleset rules, they often have an ARGS|REQUEST_BODY construct.

In this construct, the semantic idea is that you are either able to parse the request (then use the parsed part in ARGS) or you can not parse the request because of unknown content-type and then you inspect the whole REQUEST_BODY for attack patterns. This is more or less what the documented behaviour from version 2 describes and from my point of view, this is a valid and important use case.

So my question is:

How could I express this intension otherwise? I think, the options are:

we can duplicate all rules and decided depending on your body parser if you use the ARGS rules or the REQUEST_BODY rules. Thats a maintenance nightmare
we can continue to use ARGS|REQUEST_BODY and waste a lot of CPU by scanning it twice. That could be a performance problem.

Do you have another proposal? Do I miss something?

mirkodziadzka-avi on Sep 16, 2019