8000 fix: match word boundary after `fi` by theseion · Pull Request #3187 · coreruleset/coreruleset · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: match word boundary after fi #3187

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 6, 2023
Merged

fix: match word boundary after fi #3187

merged 1 commit into from
Apr 6, 2023

Conversation

theseion
Copy link
Contributor
@theseion theseion commented Apr 2, 2023

Fixes an FP in rule 932236 agains fi at the beginning of a word in argument names and values because no word boundary is required.

Also updates the unix anti evasion suffix to include ) as word boundary after a known command word.

Concerns #3172.

@theseion
fix: match word boundary after fi
4c78b1a 
Fixes an FP in rule 932236 agains `fi` at the beginning of a word in
argument names and values because no word boundary is required.

Also updates the unix anti evasion suffix to include `)` as word
boundary after a known command word.
@theseion theseion requested a review from a team April 2, 2023 15:29
@fzipi fzipi mentioned this pull request Apr 2, 2023
Closed
@emphazer emphazer requested review from emphazer and removed request for a team April 5, 2023 06:12
@emphazer
Copy link
Contributor
emphazer commented Apr 5, 2023

i will check this one...
it looks good so far but i will test it in production. so stay tuned ;-)

@dune73 dune73 mentioned this pull request Apr 5, 2023
Merged
@dune73
Copy link
Member
dune73 commented Apr 5, 2023

@emphazer Very welcome to test this in prod. Can you give us a timeframe how long we should wait with the merging, or do we merge and you report anew if this continues to be a problem (or develops into a new one)?

@emphazer
8000 Copy link
Contributor
emphazer commented Apr 5, 2023

@dune73 well, for me its always easier if the PR is merged already, because then i can use our full dev autobuild rpm package with version and changelog. its more transparent.
but in this case i could also just manually change the rule file if you prefer.
in my eyes this PR looks technically good.

if i change it manually then i want to test this PR until tommorow evening.

@dune73
Copy link
Member
dune73 commented Apr 5, 2023

Perfect. Thank you.

(And I see your reasoning.)

emphazer
emphazer approved these changes Apr 6, 2023
View reviewed changes
Copy link
Contributor
@emphazer emphazer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works perfect!

@emphazer emphazer merged commit c6c6911 into coreruleset:v4.0/dev Apr 6, 2023
@theseion theseion deleted the 3172-fp-in-932236 branch April 16, 2023 11:07
@theseion theseion mentioned this pull request Apr 23, 2023
Closed
@fzipi fzipi mentioned this pull request May 1, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emphazer emphazer emphazer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@theseion @emphazer @dune73
0