8000 fix(932100): update data list by fzipi · Pull Request #2676 · coreruleset/coreruleset · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix(932100): update data list #2676

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 2 commits into from
Sep 19, 2022
Merged

fix(932100): update data list #2676

merged 2 commits into from
Sep 19, 2022

Conversation

fzipi
Copy link
Member
@fzipi fzipi commented Jun 30, 2022 •
edited
Loading

Signed-off-by: Felipe Zipitria felipe.zipitria@owasp.org

Updated data list based on unix-shell.data.

I've also added some regexes for programming languages with versions.

Fixes #2673 .

Should have mentioned that this fixes also BB 9KO58Y4W.

@dune73 dune73 mentioned this pull request Jul 4, 2022
Closed
@fzipi fzipi force-pushed the update-932100-data-list branch from 1cc2040 to 18244b7 Compare July 16, 2022 13:31
@fzipi fzipi requested a review from theseion July 16, 2022 13:32
@fzipi
Copy link
Member Author
fzipi commented Jul 16, 2022
  • Added more tests
  • Changed test titles to match our generic naming

@fzipi fzipi force-pushed the update-932100-data-list branch from 18244b7 to 6b5669d Compare July 16, 2022 13:33
@fzipi fzipi mentioned this pull request Jul 18, 2022
Merged
@RedXanadu
Copy link
Member

Sorry, I haven't had time to look at this one either, yet. It, also, is still on my todo list.

@dune73
Copy link
Member
dune73 commented Jul 29, 2022

I think the following items would profit from @.

ash
bridge
bundler ...
column
dd
docker
finger
genie
gimp
install
latex

theseion
theseion requested changes Jul 31, 2022
View reviewed changes
Copy link
Contributor
@theseion theseion left a comment •
edited
Loading

Choose a reason for hiding this comment

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

+ things spotted by @dune73

@fzipi fzipi mentioned this pull request Aug 1, 2022
Closed
@fzipi fzipi force-pushed the update-932100-data-list branch from 6b5669d to cadeca5 Compare August 1, 2022 17:44
@fzipi
Copy link
Member Author
fzipi commented Aug 1, 2022

Addressed comments, and added more bare English words with @. Also, removed some repeated ones.

@fzipi fzipi requested a review from theseion August 1, 2022 17:45
@RedXanadu
Copy link
Member
RedXanadu commented Aug 1, 2022 •
edited
Loading

Oops, sorry: I didn't follow up my earlier comment.
After looking at this more closely, the same as with #2677: I don't think there are concerns about adding in anti-evasion patterns as there isn't a specific suffix being searched for, so it's fine (e.g. lua5"".4 won't work as an evasion as it will still match on lua alone.)

@fzipi
Copy link
Member Author
fzipi commented Aug 2, 2022
8000

In summary: adding the numbers won't give us added value in this case, so I'm reverting them back to their original positions in the file and removing the specific cases.

@fzipi fzipi force-pushed the update-932100-data-list branch 2 times, most recently from e06e0dd to f8180e3 Compare August 2, 2022 10:37
theseion
theseion reviewed Aug 3, 2022
View reviewed changes
theseion
theseion approved these changes Aug 3, 2022
View reviewed changes
Copy link
Contributor
@theseion theseion left a comment

Choose a reason for hiding this comment

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

LGTM

@lifeforms
Copy link
Member

Tests are failing now, but that could be due to a temporary problem with our pipeline that should be solved now. Re-running jobs.

@lifeforms
Copy link
Member

Two test failures persist:

running 932100-28: 💥 failed in 4.404215ms - POST data: arg=;gcc10.1
running 932100-29: 💥 failed in 4.240415ms - POST data: arg=;irb31

@fzipi
Copy link
Member Author
fzipi commented Aug 3, 2022

That happens after the last update from @RedXanadu. I've removed the suffix search. But looks like now we are not matching anymore.

@theseion
Copy link
Contributor
theseion commented Aug 3, 2022

Note that this issue is affected by the same thing as #2677. We need to figure out how we ignore python but still catch python3. I've suggested adding an additional symbol for an adjusted anti-evasion pattern in the other PR.

@fzipi fzipi mentioned this pull request Sep 5, 2022
Closed
@fzipi fzipi force-pushed the update-932100-data-list branch 3 times, most recently from 414b5a2 to 0284a90 Compare September 17, 2022 12:14
@fzipi
Copy link
Member Author
fzipi commented Sep 17, 2022

Updated:

  • irb -> irb~
  • gcc -> gcc~

@fzipi fzipi requested a review from theseion September 17, 2022 12:16
@fzipi fzipi force-pushed the update-932100-data-list branch from 0284a90 to 7788ea1 Compare September 17, 2022 12:18
@fzipi
Copy link
Member Author
fzipi commented Sep 17, 2022

🤔 Looks like they fail when using ~... but didn't failed previously. Let's try removing that.

@fzipi
fix(932100): update data list
09314fc 
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi fzipi force-pushed the update-932100-data-list branch 2 times, most recently from 6677ae0 to a42de79 Compare September 18, 2022 14:55
@theseion
Copy link
Contributor

I might be stating the obvious but you'll need to change gcc to gcc~ in the data file for the new test to work (same for irb).

@fzipi
fix(932100): add dynamic version to cmds. update tests
874b88c 
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi fzipi force-pushed the update-932100-data-list branch from a42de79 to 874b88c Compare September 19, 2022 12:08
@fzipi
Copy link
Member Author
fzipi commented Sep 19, 2022

Thanks for the tips. Looks like working now. Following approval, let's merge then.

@fzipi fzipi merged commit b616e75 into coreruleset:v4.0/dev Sep 19, 2022
@fzipi fzipi deleted the update-932100-data-list branch September 19, 2022 12:30
@fzipi fzipi mentioned this pull request Oct 2, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@theseion theseion Awaiting requested review from theseion

Assignees
No one assigned
Labels
⚡ list update
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

update word list for rule 932100 (RCE Unix command injection part 1/4)
5 participants
@fzipi @RedXanadu @dune73 @lifeforms @theseion
0