fix: 933160 regex #2301
fix: 933160 regex #2301
Conversation
|
switched to draft because a valid syntax could be something like $a = ["system"];
$a #foo
[0] //bar
('ls');so we need a different solution :( |
| @@ -24,7 +24,7 @@ | |||
|
|
|||
| ##!+i | |||
| ##!^\b | |||
| ##!$(?:\s|/\*.*\*/|//.*|#.*)*\(.*\) | |||
| ##!$\(.*\) | |||
There was a problem hiding this comment.
After removing whitespace and comment markers you might miss this:
=array_map\n#dummy comment\n('gotcha')
which would be converted to:
=array_mapdummycomment('gotcha')
Removing whitespace could be ok I guess but removing comment markers will break the rule.
There was a problem hiding this comment.
yep, your example payload doesn't match anymore :/ The problem we need to solve is just the //.* part of the regex. I'm switching this to draft while looking for a solution tnx
There was a problem hiding this comment.
Maybe you can work around it by matching the end of the line, since // will comment everything up to the end:
//[^\n\r]*[\n\r]\(.*\)
|
What is the status here? |
|
IMHO, I don't think we can solve the aliasing problem using secrules. So we should stick with something with less coverage, but functional. |
7a1d7a9 to
9f37d51
Compare
9f37d51 to
5af2739
Compare
5af2739 to
3b881e6
Compare
|
rebased on v4.0/dev |
|
it seems that regex assembly has done the magic, the produced regex is no more affected by ReDoS! but I think the rule regression tests need to be fixed... Can anyone explain why we're doing this? $ grep 'Print_r' tests/regression/tests/REQUEST-933-APPLICATION-ATTACK-PHP/933160.yaml
data: x=Print_r%28%20%29
data: x=Print_r%28%20%29
data: x=Print_r%28%20%29
data: x=Print_r%28%20%29
data: x=Print_r%28%20%29
data: x=Print_r%28%20%29
data: x=Print_r%28%20%29
data: x=Print_r%28%20%29
data: x=Print_r%28%20%29
data: x=Print_r%28%20%29
data: x=Print_r%28%20%29
data: x=Print_r%28%20%29
data: x=Print_r%28%20%29
data: x=Print_r%28%20%29maybe there's a good reason, but all these tests pass just because the rule matches always the same payload. |
|
better opening an issue for this. |
|
Ha, nice :D |
|
I agree on the tests. They're garbage. |
removed suffix
#*and//*from regex since isn't a valid PHP syntaxthe only valid syntax should be