append data to variables
MODSEC-187: Like adding number to a variable (TX.setvar: var=+1), it would be useful to be able to append a string to a variable (ex: TX.setvar:var=>new_string).
In usual cases, this could oviously be done manually: TX.setvar:var=%{TX.setvar}new_string
But you cannot do the same when you variable name depends on a variable: TX.setvar:var_%{TX.number}=>new_string
Original reporter: marcstern
rbarnett: How about adding macro expansion to the + setvar action? So you could so something like this -
setvar:tx.foo=+%{matched_var}
So, if tx.foo already had data in it, it would append the MATCHED_VAR data to it. Essentially, we would need to add macro support and make sure that the + and - variables understand the different between adding/subtracting digits vs. appending/removing macro string data.
marcstern: Macro expansion is already working inside setvar/setenv... I don't really see the point about expansion, except if you need to move the parsing at run-time instead of config time.
Any way, I think that an explicit "append" operator is safer than a "magic" one determining if the string is a number or not. If I want to append a string, it may contain a number...
marcstern: Too bad we have to wait for 3.0, as this functionality can be used to block HTTP parameter pollution attacks :-(
bpinto: It can be changed Marc, we are hiring a new devel to the team :) Anyway i will reschedule this. Maybe for some 2.7.x.
bpinto: Marc,
This is not clear to me. Could you please give me more details ? maybe some real situations and an example of SecRule.
Thanks
marcstern: Here is a (unoptimised) example to detect HPP. We want to concatenate all args with the same name into one. Ex: var=1&var=2 => HPP_var=1,2
Same rule as CRS 960022, but for all args.
This would be something like
SecRule ARGS "."
"phase:2,setvar:'TX.HPP_%{MATCHED_VAR_NAME}=>%{TX.HPP_%{MATCHED_VAR_NAME}},%{MATCHED_VAR}'"
Note the double (recursive) macro replacement: %{TX.HPP_%{MATCHED_VAR_NAME}} I don't expect this to be supported. However, by having a concatenation operator similar to the '+' (ex: '>'), we could write it:
setvar:'TX.HPP_%{MATCHED_VAR_NAME}=>,%{MATCHED_VAR}'
bpinto: Ok Marc,
I would suggest actions attach and detach
attach:var_name;value detach:var_name;value
or
string-set:var+=value string-set:var-=value
marcstern: 1. attach/detach does not sound very intuitive to me
- I am not sure about the "detach" feature. Is it to remove something from the string? I do not see the use case.
- the string-set syntax is not consistent with the setvar one. "string-setvar:var=+value" would be more in-line or even more intuitive: "appendvar:var=value"
- what about simply adapting the current syntax to make it generic (and unambiguous)?
- adding a number: setvar:var+=value
- substracting a number: setvar:var-=value
- appending a string: setvar:var>=value
- prepending a string: setvar:var<=value
- possibly others in the future ... "=+" and "=-" would stay for backward compatibility (for some time)
and the same could be done for setenv (2 birds with one stone)
bpinto: I like the <= and >=. Sounds good. I will let you know when i have updates about it.
thanks
rbarnett: why not keep the same syntax/placement for the modifiers?
- adding a number: setvar:var=+value
- substracting a number: setvar:var=-value
- appending a string: setvar:var=>value
- prepending a string: setvar:var=<value
bpinto: Yes. must use the same logic.
marcstern: The idea behind putting the character before the = sign instead of after was because you can always chose the variable name, but not the content. You may really want to set/append/prepend a string containing the + character at the first place. And, the more operator characters you add (currently + - < > but maybe others in the future), the more chance you have to get a conflict with a value you need to use.
I just thing it would be a safer choice, but it is obviously functionally equivalent.
Any chance to implement this? Issue #903 is another way to solve HTTP parameter pollution but this could be a very interesting generic feature
Setvar with with macro expansion should be enough to achieve such functionality. I cannot foresee a use case scenario where a limitation is imposed.
From my example above:
SecRule ... "setvar:'TX.HPP_%{MATCHED_VAR_NAME}=%{TX.HPP_%{MATCHED_VAR_NAME}},%{MATCHED_VAR}'"
This would require double (recursive) macro replacement: %{TX.HPP_%{MATCHED_VAR_NAME}}
This obviously doesn't work.
Note that the first part works (TX.HPP_%{MATCHED_VAR_NAME}=).
However, by having a concatenation operator similar to the '+' (ex: '>'), we could write it:
setvar:'TX.HPP_%{MATCHED_VAR_NAME}=>,%{MATCHED_VAR}'
From my example above:
SecRule ... "setvar:'TX.HPP_%{MATCHED_VAR_NAME}=%{TX.HPP_%{MATCHED_VAR_NAME}},%{MATCHED_VAR}'"This would require double (recursive) macro replacement: %{TX.HPP_%{MATCHED_VAR_NAME}} This obviously doesn't work. Note that the first part works (TX.HPP_%{MATCHED_VAR_NAME}=).However, by having a concatenation operator similar to the '+' (ex: '>'), we could write it:
setvar:'TX.HPP_%{MATCHED_VAR_NAME}=>,%{MATCHED_VAR}'
What is the use case for setting a key based in a variable, not the value?
From my example above:
SecRule ... "setvar:'TX.HPP_%{MATCHED_VAR_NAME}=%{TX.HPP_%{MATCHED_VAR_NAME}},%{MATCHED_VAR}'"This would require double (recursive) macro replacement: %{TX.HPP_%{MATCHED_VAR_NAME}} This obviously doesn't work. Note that the first part works (TX.HPP_%{MATCHED_VAR_NAME}=). However, by having a concatenation operator similar to the '+' (ex: '>'), we could write it:setvar:'TX.HPP_%{MATCHED_VAR_NAME}=>,%{MATCHED_VAR}'What is the use case for setting a key based in a variable, not the value?
@martinhsv pointed me to a valid use case scenario for this one. I understand its value. Working on it.
Implemented at 95dc23a5aa30e659ed312999b45808a393e92e28. Code is on QA. Still need some love. Definitively needs better test cases.
With that: https://github.com/SpiderLabs/ModSecurity/blob/95dc23a5aa30e659ed312999b45808a393e92e28/test/test-cases/regression/action-setvar.json#L30-L34
We expect to have this: https://github.com/SpiderLabs/ModSecurity/blob/95dc23a5aa30e659ed312999b45808a393e92e28/test/test-cases/regression/action-setvar.json#L6-L9
New test cases at b8478b1
That's great. Any chance to have it in 2.9.x?