Owasp-modsecurity-crs: Rule idea: Multiple parameters with the same name

[dune73]

Can we do a rule that checks if a parameter name has been submitted multiple times? I think this should be possible with a counter as TX:paramcounter_<param-name> and then check @gt on TX:/^paramcounter_.*/.

Naturally, this would be a PL3 or PL4 rule.

[csanders-git]

If i remember correctly the answer is no .... However, if we did it would probably have different functionality on each web server. I believe Apache did something weird like comma separate them

i.e test.com?x=123&x=456 = Apache x=123,456
i'd have to check - which i will right now since its 12:30AM and what else would i do :-D

[csanders-git]

So apparently i'm just wrong and i'm not quite sure what i'm remembering but whatever it is... it's not relevant to this conversation :-P ...

[csanders-git]

So to answer your question though it is EVEN easier than you might think. Even though it doesn't really make as much sense as we would have hoped...
Given my previous example of 'test.com?x=123&x=456

The rule
SecRule &ARGS_GET:x "@rx (.*)" "id:122,capture,msg:'%{tx.0}--Testing Msg',pass"

Will return only once with the amount of elements of that name that are specified. (two in this case) Which is actually kinda interesting in its own right.

so this would have made it easy but alas no such luck luckhttps://github.com/SpiderLabs/ModSecurity/issues/1131

^^^This was my thought ^^^
SecRule ARGS_NAMES "@rx (.*)" "id:123,capture,chain,msg:'did this work'"
SecRule &ARGS:%{TX:0} "@gt 1" "t:none"

@zimmerle this functionality is probably not working correctly in v3 since it's kinda unintuative (not sure & has been implemented yet either)

[csanders-git]

and yes the counter approach would work probably but this approach would be SOOO much nicer

[marcstern]

This should do the trick:

  SecRule ARGS_NAMES "."

"phase:2,id:1,nolog,chain,t:none,capture,setvar:'TX.%{MATCHED_VAR_NAME}=+1'"
  SecRule TX:/ARGS_NAMES:.*/ "@gt 1"
"capture,setvar:TX.http_parameter_pollution=%{MATCHED_VAR_NAME}"
SecRule TX:http_parameter_pollution "."
"phase:2,id:2,deny,t:none,msg:'Multiple Parameters with the same
Name',logdata:'%{MATCHED_VAR}'"

The principle is working but I had to transform an existing rules,
so some syntax tweaks may be needed


On 12-07-2016 06:34, Christian Folini
  wrote:



  Can we do a rule that checks if a parameter name has been
    submitted multiple times? I think this should be possible with a
    counter as TX:paramcounter_ and then check @gt on TX:/^paramcounter_.*/.
  Naturally, this would be a PL 3 or PL4 rule.
  —
    You are receiving this because you are subscribed to this
    thread.
    Reply to this email directly, view
      it on GitHub, or mute
      the thread.

[dune73]

@csanders-git I think your approach does not work, because TX.0 is always the last match and your first rule in the chain is first executed for all ARGS_NAMES and _then_ the second rule in the chain is executed for all ARGS_NAMES that matched on the first rule with the last ARGS_NAME which matched in TX.0.

@marcstern Exactly the idea I had in mind. Thank you for the proof of concept of the idea.
I think the rule with 2 is not really necessary, or is it. (Good to read you here. I was not aware you were following the CRS.)

[marcstern]

It's not because I'm not USING the CRS that I'm not following them
;-)

On 12-07-2016 08:58, Christian Folini
  wrote:



  @csanders-git
    I think your approach does not work, because TX.0 is always the
    last match and your first rule in the chain is first executed
    for all ARGS_NAMES and then the second rule in the
    chain is executed for all ARGS_NAMES that matched on the first
    rule with the last ARGS_NAME which matched in TX.0.
  @marcstern Exactly the idea I had in
    mind. Thank you for the proof of concept of the idea. 
    I think the rule with 2 is not really necessary, or is it. (Good
    to read you here. I was not aware you were following the CRS.)
  —
    You are receiving this because you were mentioned.
    Reply to this email directly, view
      it on GitHub, or mute
      the thread.

[lifeforms]

I'd absolutely love a rule like this. I wonder how many real world applications actually send duplicate parameters? I wouldn't mind trying to detect this at a low paranoia level.

[dune73]

If it is low on FPs, then why not. Will do a PR.

[rcbarnett-zz]

HTTP Param Pollution PoC -
https://github.com/SpiderLabs/owasp-modsecurity-crs/blob/master/experimental_rules/modsecurity_crs_40_http_parameter_pollution.conf

It works for single HPP attack but if there are multiple params with different names (foo.asp?bar=1&bar=2&baz=1&baz=2) then it would only trigger the last match (multiple "baz" params) as the variables overwrite themselves.

[dune73]

Thanks for the link @rcbarnett. Should have looked at the experimental rules more. ;)

Can't we cut away all the decoration and run with a construct like the following:

SecRule ARGS_NAMES "." "phase:2,id:1,nolog,setvar:'TX.paramcounter_%{MATCHED_VAR_NAME}=+1'"   

SecRule TX:/paramcounter_.*/ "@gt 1" "phase:2,id:2,pass,msg:'HTTP Parameter Pollution (counter name: %{MATCHED_VAR_NAME})'"

For 2 params each submitted twice, this results in the following messages:

HTTP Parameter Pollution (counter name: TX:paramcounter_ARGS_NAMES:bar)
HTTP Parameter Pollution (counter name: TX:paramcounter_ARGS_NAMES:foo)

I am running with a prefix "paramcounter_". This is optional of course and the name arbitrary, however, I would like to avoid a variable like TX:ARGS_NAMES:foo with a value as a counter. (a) it's not obvious what it is (if you do not know the rule) and (b) people could run into conclusions by using this variable key for their own purposes.

The way I see it, ModSec is standing in our way to remove the TX:paramcounter_ from the message. It's always the problem that in a chained rule with multiple hits on the first rule, the 2nd rule does not know the corresponding first match anymore.

[dune73]

UPDATE: In this particular case, I have found a way around the SecRule-Chain-Alzheimer:

SecRule ARGS_NAMES "." "phase:2,id:1,nolog,setvar:'TX.paramcounter_%{MATCHED_VAR_NAME}=+1'"   

SecRule TX:/paramcounter_.*/ "@gt 1" "phase:2,chain,id:2,pass,msg:'HTTP Parameter Pollution (%{TX.1})'"
    SecRule MATCHED_VARS_NAMES "TX:paramcounter_(.*)" "capture"

Resulting in

HTTP Parameter Pollution (ARGS_NAMES:bar)
HTTP Parameter Pollution (ARGS_NAMES:foo)

[dune73]

Ran this against my local test traffic.

I got quite a few FPs. On google maps (where it might be legitimate) for example and on a random news site, where it looks like a bug in a link. All in all single digit percentage of FPs on legitimate traffic.

My attack traffic also got positives. Very few ones, though, likely because this attack class is not actively exploited by my attack traffic.

So I would say PL1 or 2 are out, but let's put this into PL3 for CRS3 and see how it goes.
(Unless of course the proposed rule does not work as advertised because I overlooked something.)

Thoughts?

[csanders-git]

Wow this blew up overnight :)... What can i say a good idea spreads like fire fire...I'd say PL3 seems good. I'm slightly concerned about overhead from setting all the vars but i'll run it through the timing tests - in any event it'd be a nice PR.

[dune73]

The overhead was another reason I would prefer to keep this at a higher PL. ;)

[lifeforms]

Great!

[lifeforms]

Discussion will move to #472!