fix: incorrect id for 932230-58 #4018
Conversation
The expect id for the test 932230-58 was set to 932130, when in fact it should be 932230. It is more than probable that it was copy pasted from the test for 932130 but @Xhoenix forget to update it. Relevant pr: coreruleset#3756
|
Thanks @TimDiam0nd! Unfortunately, the test now fails. |
|
Yep i did notice that. I did try looking into why it did not match, but unfortunately could find anything. Considering it is a duplicate of 932130-17, would you prefer I remove this? |
|
I think doing the same arithmetic expansion check in two places, i.e |
4ba77ed to
6529da6
Compare
|
📊 Quantitative test results for language: |
|
@Xhoenix done (i think) |
|
Is there any consequence on additional rules? This affects 932230, but also 932235, 932250, 932260, 932231, 932220, 932236, 932239, 932232 and 932238. No changes on params, transformation, anything particular that might be affected and not catched by 932130? I would say we probably add tests to 932130 to be sure that we catch all the possible permutations or problems catches with the above rules to just be sure we are covered? Also, as a followup Q: would rule 932130 catch additional evasion techniques that we might want to move away from the above rules to consolidate/simplify them? |
|
The rules are affected because they are indirectly using |
|
I agree. But can we be sure we are covered by adding tests? |
|
Followup Q: is there any other prefix that we might benefit to remove if this is the case (maybe not for this PR, but I'm thinking on followups 😉 ) |
|
I'm not comfortable with that change. I'd rather you modify the test to text arithmetic expansion for rule 932230 properly. |
I believe that is already there: coreruleset/regex-assembly/include/932130.ra Lines 10 to 11 in 941bb73
There is already a test in 932130 for the payload:
i.e. instead of removing the test from 932230, change it to |
|
No, adjusting the payload so it triggers the rule: - test_id: 58
desc: |
Bash - Arithmetic expansion ($[...]).
Payload: $[$(sh -c 'echo 1')+2]
stages:
- input:
dest_addr: "127.0.0.1"
method: "GET"
port: 80
headers:
User-Agent: "OWASP CRS test agent"
Host: "localhost"
Accept: text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,ima>
uri: "/get?932230-58=$%24%5B%24(sh%20-c%20'echo%201')%2B2%5D"
version: "HTTP/1.1"
output:
log:
expect_ids: [932230] |
|
@TimDiam0nd Any updates on this? |
|
Hey @Xhoenix sincere apologies for the delay on that, totally slipped my mind |
|
@TimDiam0nd Can you revert the other changes, as the test is now correctly triggering rule |
73e3b14 to
36204e4
Compare
|
Thanks @TimDiam0nd. |
The expect id for the test 932230-58 was set to 932130, when in fact it should be 932230.
It is more than probable that it was copy pasted from the test for 932130 but @Xhoenix possibly forgot to update it the expect ids (pr).