-
-
Notifications
You must be signed in to change notification settings - Fork 402
Amend regular expression pattern for rule 942440 #2201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This PR is intended to resolve the issue with the regular expression pattern with rule 942440, as discussed in issue #2179 |
Added an additional test for rule 942440: ensure that the URL encoded null byte example input is correctly caught |
If it makes sense, could you break up the expression and move it to a data file so we can generate it? |
We can also do an issue to make this as a separate step. But I agree, this rule ought to have a source-file with the individual regexes and we generate it. It might also become faster / optimized that way. |
Okay, so can we look at that in a separate issue, then? IMO issue #2179 is already too complex/broad (that's my fault). |
Sure, put it in a separate issue. |
No worries @RedXanadu. We're happy you are solving it, step by step. |
Okay: I looked into it and realised that Could someone check the commits, please? I think I understand how to use I did notice that (P.S. The |
@RedXanadu I just glanced at the changes for I've opened #2210 to fix the issue. Does the test work when the expression contains a |
Thank you @theseion. It looked like a bug, but I wasn't certain: thanks for opening one :)
I'm not sure I understand the question. Do you mean, does rule 942440 work correctly if the regular expression contains the literal null character that ModSecurity refuses to start in that scenario:
|
Yes. That's ok then. leave it like you have it know and I'll clean it up once the issue is fixed. |
The failing tests will probably succeed when you rebase the PR on v3.4/dev, could you try that please? |
Meeting decision Oct 4: a rebase should fix the failing tests. |
This should make the regular expression have the same end result with both Apache/mod_security2 and nginx/libmodsecurity
Adds a test for a SQL comment ending with a URL encoded null character. Historically, mod_security2 on Apache would catch such input but libmodsecurity would not: this test should now pass on both platforms
c9c7cf4
to
3a050fc
Compare
I believe this is good to go, and can be simplified once the new regex assembler is in place (for now, manual modifications are required, so the documentation on how to do that should stay for now). |
I think so too. |
Excellent, I concur. Merging then. |
This should make the regular expression have the same end result with
both Apache/mod_security2 and nginx/libmodsecurity