SpiderLabs / owasp-modsecurity-crs

OWASP ModSecurity Core Rule Set (CRS) Project (Official Repository)
https://modsecurity.org/crs
Apache License 2.0
2.45k stars 724 forks source link

Rule 942350: False positive #1587

Closed Taiki-San closed 4 years ago

Taiki-San commented 4 years ago

Type of Issue

False positive

Description

The rule incorrectly detect patterns like ;insertion_424242. I suspect this is because the regex doesn't require spaces (\s*) before matching an arbitrary number of \w (\w{2,}). The obvious fix (\s+) is likely to fail because of test vectors, I'll need to have a deeper look. I'm thinking of updating the insert regex (and maybe others) with the following pattern \s*?insert(\s+|\s*?[\[(]\w{2,}), starting from \s*?insert\s*?[\[(]?\w{2,}

Confirmation

[X] I have removed any personal data (email addresses, IP addresses, passwords, domain names) from any logs posted.

github-actions[bot] commented 4 years ago

This issue has been open 120 days with no activity. Remove the stale label or comment, or this will be closed in 14 days

Taiki-San commented 4 years ago

The issue still exists, not sure tagging it as stale and closing it is the right way forward :)

Taiki-San commented 4 years ago

Hey, the issue shouldn't have been closed as it's still relevant. Can someone open it up again?

dune73 commented 4 years ago

Yes. Added to the agenda for Monday.

@Taiki-San: We're meeting at 20:30 your time on https://owasp.slack.com in the channel #coreruleset. It would be nice to have you around as well.

Taiki-San commented 4 years ago

Thanks! Will try to be there but I'm flying back from the US at roughly midnight CET. Might miss the beginning.

franbuehler commented 4 years ago

I think we could add a word boundary after insert.

Original regex: ;\s*?insert\s*?[\[(]?\w{2,} New regex: ;\s*?insert\b\s*?[\[(]?\w{2,}

And I would apply this \b to every sql word in the source file regexp-942350.data expect the first entry:

create\s+function\s+.+\s+returns
;\s*?alter\b\s*?[\[(]?\w{2,}
;\s*?create\b\s*?[\[(]?\w{2,}
;\s*?delete\b\s*?[\[(]?\w{2,}
;\s*?desc\b\s*?[\[(]?\w{2,}
;\s*?insert\b\s*?[\[(]?\w{2,}
;\s*?load\b\s*?[\[(]?\w{2,}
;\s*?rename\b\s*?[\[(]?\w{2,}
;\s*?select\b\s*?[\[(]?\w{2,}
;\s*?truncate\b\s*?[\[(]?\w{2,}
;\s*?update\b\s*?[\[(]?\w{2,}

I will make a PR for that.

Taiki-San commented 4 years ago

Thanks! Indeed look more appropriate than my suggestion

franbuehler commented 4 years ago

This issue has been solved by PR #1706