8000 fix(sqli): additional regular expressions for SQL auth bypass by karelorigin · Pull Request #2557 · coreruleset/coreruleset · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix(sqli): additional regular expressions for SQL auth bypass #2557

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 7 commits into from
Jul 11, 2022
Merged

fix(sqli): additional regular expressions for SQL auth bypass #2557

merged 7 commits into from
Jul 11, 2022

Conversation

karelorigin
Copy link
Member
@karelorigin karelorigin commented May 15, 2022 •
edited
Loading

This PR contains new regular expressions for different kinds of auth bypasses. In an ideal world, these would be inside the operator dataset, but unfortunately, they look too much like normal sentences to accurately block.

Edit:

  • Contains fixes to bypasses found by me (DV2UKLP4, 2AUWUOVF, RICGZH4Q)
  • Contains fixes to bypasses found by @wfinn (YPWZU6PD and Q6FBUFOD)

@fzipi fzipi added the ⭐ bug bounty Comes from our Bug Bounty program label May 15, 2022
@karelorigin karelorigin mentioned this pull request May 15, 2022
Closed
@dune73 dune73 mentioned this pull request May 16, 2022
Closed
14 tasks
@karelorigin karelorigin force-pushed the 942340-patch-DV2UKLP4 branch 2 times, most recently from 406d8e1 to f232001 Compare May 16, 2022 07:11
@azurit
Copy link
Member
azurit commented May 16, 2022

Hi @karelorigin , thanks for sending this!

@karelorigin karelorigin marked this pull request as ready for review May 17, 2022 11:31
@karelorigin
Copy link
Member Author

Hi!

This PR should also be able to catch a NoSQL bypass reported by @hussein98d in RQ1R4RCP, except that it's not evaluated on the URL path. Is this something I could add?

@dune73
Copy link
Member
dune73 commented May 20, 2022

This also covers DV2UKLP4

@karelorigin
Copy link
Member Author

Hi @dune73, I've updated my original comment to better outline the reports this PR applies to. That should be the complete list of reports relevant to this PR.

@dune73
Copy link
Member
dune73 commented May 20, 2022

Thank you. We'll get to the question of RQ1R4RCP during the review.

@dune73
Copy link
Member
dune73 commented May 20, 2022 •
edited by lifeforms
Loading

CRS Bug Bounty PR assessment

  • Rules affected (list rules): 942340
  • Paranoia Level addressed (1, 2, 3, 4, full or explain): 2
  • FTW passes (yes or no) : Yes
  • Rule(s) picked for solution (correct or not-correct or explain) : correct
  • Risk for false positives (irrelevant, adequate, substantial or explain) : adequate
  • Regular expression quality (inspirational, decent base, needs work, adequate or explain) : adequate
  • Documentation (needs work, adequate or explain) : adequate
  • Tests (none or some or adequate) : adequate
     
  • Verdict (Unusable, inspirational, usable, almost perfect or perfect) : almost perfect

This is not meant to be final. As a CRS dev, feel free to comment below and edit this form directly. As committer or ob 8000 server, feel free to comment below with feedback and we will think about updating the assessment accordingly.

@fzipi
Copy link
Member
fzipi commented May 21, 2022

CRS TODO: add a comment on file util/regexp-assemble/data/942340.data line 43-47 on why are those there, so documentation is in one file (we can't ask this in this PR, wasn't agreed in the first place).

@fzipi fzipi changed the title Additional regular expressions for SQL auth bypass fix(sqli): additional regular expressions for SQL auth bypass Jun 4, 2022
@fzipi
Copy link
Member
fzipi commented Jun 5, 2022

Hi @karelorigin !

Inevitably, we run onto conflicts after we started merging. Can you help us resolve this one?

@fzipi fzipi mentioned this pull request Jun 5, 2022
Closed
@karelorigin
Copy link
Member Author

Hey @fzipi, yeah for sure! I could rebase but that rewrites git history and will probably create a mess, so I'll try to do it via the GitHub interface. Will work on it as soon as I get the chance (might take a couple of days)

@lifeforms
Copy link
Member

Hi @karelorigin, there seem to be still some odd conflicts, could you solve them or do you need help?

@karelorigin
Copy link
Member Author

@lifeforms, I've been super busy but I'll get on this ASAP. I should be able to resolve them just fine but I'll definitely ask for help if I need it. Sorry about that!

@dune73 dune73 mentioned this pull request Jul 4, 2022
Closed
@karelorigin karelorigin force-pushed the 942340-patch-DV2UKLP4 branch from a00e94a to ffaa90f Compare July 9, 2022 16:22
@karelorigin
Copy link
Member Author

Resolved all conflicts, should be fine merging this now :)

@fzipi
Copy link
Member
fzipi commented Jul 11, 2022

Looks awesome! Thanks @karelorigin for this PR! 🎉

@fzipi fzipi merged commit 6ac4f67 into coreruleset:v4.0/dev Jul 11, 2022
@fzipi fzipi mentioned this pull request Aug 1, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@fzipi fzipi

Labels
👀 Needs action 🖊️ CRS Internal Note ⭐ bug bounty Comes from our Bug Bounty program
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@karelorigin @azurit @dune73 @fzipi @lifeforms
0