fix: removing double t:urlDecodeUni (920221 PL1, 920440 PL1, 932200 PL2, 932205 PL2, 932206 PL2)

[azurit]

Removing double t:urlDecodeUni transformation from rules 920221, 920440, 932200, 932205 and 932206. This is NOT related to #3297.

Removing excessive / not used capture action from rules 933120 and 933151. Operator @pm replaced with @contains.

[azurit]

@theseion Do we really want to double URL decode in rule 920221? Is /get/%25w20 a real payload as it needs to be URL decoded twice?

[theseion]

I think you're right, the double decoding is wrong there. Decoding twice wouldn't change the result.

[azurit]

I think you're right, the double decoding is wrong there. Decoding twice wouldn't change the result.

It is probably changing the result as, after removing the second URL decode, one of the tests is not passing.

[theseion]

Removing the t:urlDecodeUni from the base rule leads to the following output:

[15/May/2024:05:15:07.847629 +0000] [localhost/sid#ffff90a68ea0][rid#ffff90dc00a0][/get/%25w20][4] Executing operator "!rx" with param "^.*%.*\\.[^\\s\\x0b\\.]+$" against REQUEST_BA
SENAME.
[15/May/2024:05:15:07.847712 +0000] [localhost/sid#ffff90a68ea0][rid#ffff90dc00a0][/get/%25w20][9] Target value: "%25w20"
[15/May/2024:05:15:07.847798 +0000] [localhost/sid#ffff90a68ea0][rid#ffff90dc00a0][/get/%25w20][4] Operator completed in 2 usec.
[15/May/2024:05:15:07.847880 +0000] [localhost/sid#ffff90a68ea0][rid#ffff90dc00a0][/get/%25w20][4] Rule returned 1.
[15/May/2024:05:15:07.847962 +0000] [localhost/sid#ffff90a68ea0][rid#ffff90dc00a0][/get/%25w20][9] Match -> mode NEXT_RULE.
[15/May/2024:05:15:07.848045 +0000] [localhost/sid#ffff90a68ea0][rid#ffff90dc00a0][/get/%25w20][4] Recipe: Invoking rule ffff8fed4220; [file "/etc/modsecurity.d/owasp-crs/rules/REQU
EST-920-PROTOCOL-ENFORCEMENT.conf"] [line "430"].
[15/May/2024:05:15:07.848128 +0000] [localhost/sid#ffff90a68ea0][rid#ffff90dc00a0][/get/%25w20][5] Rule ffff8fed4220: SecRule "TX:0" "@validateUrlEncoding " "t:none,setvar:tx.inboun
d_anomaly_score_pl1=+%{tx.critical_anomaly_score}"
[15/May/2024:05:15:07.848213 +0000] [localhost/sid#ffff90a68ea0][rid#ffff90dc00a0][/get/%25w20][4] Transformation completed in 1 usec.
[15/May/2024:05:15:07.848295 +0000] [localhost/sid#ffff90a68ea0][rid#ffff90dc00a0][/get/%25w20][4] Executing operator "validateUrlEncoding" with param "" against TX:0.
[15/May/2024:05:15:07.848378 +0000] [localhost/sid#ffff90a68ea0][rid#ffff90dc00a0][/get/%25w20][9] Target value: "/get/%25w20"

As you can see, REQUEST_BASENAME is not decoded, which surprises me, after all the discussions we've had around URL decoding. But at least I wrote the rule correctly :)

The t:urlDecodeUni in the chain rule, however, is probably unnecessary, since the capture will contain the decoded value already.

[azurit]

I have NOT removed t:urlDecodeUni from the base rule, only form chained rule, just see this PR.

[Xhoenix]

Looks good to me. LGTM.

[theseion]

I looked at the rule once more. This is the output (with extra logging of TX.0 in logdata) of the original rule and test:

[15/May/2024:19:05:57.424026 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][4] Recipe: Invoking rule ffff8f2ee910; [file "/etc/modsecurity.d/owasp-crs/rules/REQU
EST-920-PROTOCOL-ENFORCEMENT.conf"] [line "427"] [id "920221"].
[15/May/2024:19:05:57.424108 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][5] Rule ffff8f2ee910: SecRule "REQUEST_BASENAME" "!@rx ^.*%.*\\.[^\\s\\x0b\\.]+$" "ph
ase:1,log,tag:modsecurity,id:920221,block,capture,t:none,t:urlDecodeUni,msg:'URL Encoding Abuse Attack Attempt',logdata:'%{REQUEST_BASENAME} %{TX.0}',tag:application-multi,tag:language-multi,tag:platform-multi,tag:attack-protocol,tag:paranoia-level/1,tag:OWASP_CRS,tag:capec/1000/255/153/267/72,ver:OWASP_CRS/4.3.0-dev,severity:CRITICAL,chain"
[15/May/2024:19:05:57.424196 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][9] T (0) urlDecodeUni: "%w20"
[15/May/2024:19:05:57.424278 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][4] Transformation completed in 84 usec.
[15/May/2024:19:05:57.424363 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][4] Executing operator "!rx" with param "^.*%.*\\.[^\\s\\x0b\\.]+$" against REQUEST_BASENAME.
[15/May/2024:19:05:57.424447 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][9] Target value: "%w20"
[15/May/2024:19:05:57.424537 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][4] Operator completed in 1 usec.
[15/May/2024:19:05:57.424620 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][4] Rule returned 1.
[15/May/2024:19:05:57.424703 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][9] Match -> mode NEXT_RULE.
[15/May/2024:19:05:57.424787 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][4] Recipe: Invoking rule ffff8f2ec7b0; [file "/etc/modsecurity.d/owasp-crs/rules/REQUEST-920-PROTOCOL-ENFORCEMENT.conf"] [line "430"].
[15/May/2024:19:05:57.424876 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][5] Rule ffff8f2ec7b0: SecRule "TX:0" "@validateUrlEncoding " "t:none,t:urlDecodeUni,setvar:tx.inbound_anomaly_score_pl1=+%{tx.critical_anomaly_score}"
[15/May/2024:19:05:57.424959 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][9] T (0) urlDecodeUni: "/get/%w20"
[15/May/2024:19:05:57.425040 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][4] Transformation completed in 81 usec.
[15/May/2024:19:05:57.425121 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][4] Executing operator "validateUrlEncoding" with param "" against TX:0.
[15/May/2024:19:05:57.425203 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][9] Target value: "/get/%w20"
[15/May/2024:19:05:57.425283 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][4] Operator completed in 0 usec.
[15/May/2024:19:05:57.425369 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][9] Setting variable: tx.inbound_anomaly_score_pl1=+%{tx.critical_anomaly_score}
[15/May/2024:19:05:57.425454 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][9] Recorded original collection variable: tx.inbound_anomaly_score_pl1 = "0"
[15/May/2024:19:05:57.425540 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][9] Resolved macro %{tx.critical_anomaly_score} to: 5
[15/May/2024:19:05:57.425624 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][9] Relative change: inbound_anomaly_score_pl1=0+5
[15/May/2024:19:05:57.425709 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][9] Set variable "tx.inbound_anomaly_score_pl1" to "5".
[15/May/2024:19:05:57.425791 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][9] Resolved macro %{REQUEST_BASENAME} to: %25w20
[15/May/2024:19:05:57.425923 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][9] Resolved macro %{TX.0} to: /get/%25w20
[15/May/2024:19:05:57.426011 +0000] [localhost/sid#ffff8ffcaea0][rid#ffff90a440a0][/get/%25w20][2] Warning. Invalid URL Encoding: Non-hexadecimal digits used at TX:0. [file "/etc/modsecurity.d/owasp-crs/rules/REQUEST-920-PROTOCOL-ENFORCEMENT.conf"] [line "427"] [id "920221"] [msg "URL Encoding Abuse Attack Attempt"] [data "%25w20 /get/%25w20"] [severity "CRITICAL"] [ver "OWASP_CRS/4.3.0-dev"] [tag "modsecurity"] [tag "application-multi"] [tag "language-multi"] [tag "platform-multi"] [tag "attack-protocol"] [tag "paranoia-level/1"] [tag "OWASP_CRS"] [tag "capec/1000/255/153/267/72"]

The issue is that TX.0 contains the untransformed value of REQUEST_BASENAME. This is because the rule uses a negative match. This is certainly a bit unexpected, but probably not a bug.

In conclusion: both transformations are required for the rule to work.

[theseion]

I have NOT removed t:urlDecodeUni from the base rule, only form chained rule, just see this PR.

No, but I did when I played around with it to see what the effect was.

[Xhoenix]

@theseion can you kindly check the latest commit.

[azurit]

@Xhoenix Thanks but i'm able to update my own PRs by myself.