-
-
Notifications
You must be signed in to change notification settings - Fork 415
fix: 932270 FP #3917
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
fix: 932270 FP #3917
Conversation
@theseion using the cmdline processor for word boundary makes crs-toolchain throw error. Can you look into this. |
Can you give me an example that I can run? |
I updated the 932270.ra file with the following content, and
Is this syntax even right, this is what mentioned in our docs. Something about this doesn't seem right to me. How do we put a word boundary on the number. |
Works for me with version 2.3.0: crs-toolchain regex generate - <<EOF
##!> cmdline unix
heredoc> ~\+$
heredoc> ~\+[\d\s]+
heredoc> ~\-$
heredoc> ~\-[\d\s]+
heredoc> ~\d+~
heredoc> ##!<
heredoc> EOF
(?:\+|\x5c-)(?:$|[\s\x0b0-9]+)|[0-9]+ |
Just so we're talking about the same thing, |
The issue is with |
What do you mean by " |
Don't know, but maybe you forgot this: https://coreruleset.org/docs/development/regex_assembly/index.html#description The ~ is a special token, to prevent issues like this. If this won't work here, can you read the issue and suggest a solution? |
Right. Now I know what you mean. I still don't see the problem. On my machine, that snippet works. It also works in the CI pipeline, otherwise you'd see an error in the linting step. |
Why are you "Quote Replying" my message, it makes it hard to read your reply. Don't worry, I'm not going to delete my message. Check the latest commit, |
This is what exactly what I'm getting locally: https://github.com/coreruleset/coreruleset/actions/runs/11773621103/job/32790821426?pr=3917#step:12:82 |
That was a mistake. You're right, it doesn't work. I didn't see it when I tested it because I ran it in a terminal where the toolchain couldn't find the configuration file, so it didn't load any of the evasion replacement strings. The reason it doesn't work is that You'll have to write your own regular expression for what you intended to achieve with |
Cool. I'll be watching the progress of the issue. |
It won't change anything. When I implement that change to the toolchain, the |
I replicated the problemi with this curl command:
I tested the configuration reported into the PR:
The curl command result was OK (404 was correct because the page didn't exist):
|
📊 Quantitative test results for language: |
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
Ping |
tests/regression/tests/REQUEST-932-APPLICATION-ATTACK-RCE/932270.yaml
Outdated
Show resolved
Hide resolved
tests/regression/tests/REQUEST-932-APPLICATION-ATTACK-RCE/932270.yaml
Outdated
Show resolved
Hide resolved
tests/regression/tests/REQUEST-932-APPLICATION-ATTACK-RCE/932270.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there. Just a couple of tests to add.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Xhoenix, I know this took a long time.
Fixes #3844