8000 fix(sqli): additional sqli auth bypasses by rekter0 · Pull Request #2575 · coreruleset/coreruleset · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix(sqli): additional sqli auth bypasses #2575

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 6 commits into from
Aug 5, 2022
Merged

fix(sqli): additional sqli auth bypasses #2575

merged 6 commits into from
Aug 5, 2022

Conversation

rekter0
Copy link
Contributor
@rekter0 rekter0 commented May 17, 2022 •
edited by dune73
Loading

fixes related to 8PPEUO5P and partial for 5UXE4RK0

@RedXanadu RedXanadu added the ⭐ bug bounty Comes from our Bug Bounty program label May 17, 2022
@dune73
Copy link
Member
dune73 commented May 20, 2022 •
edited by lifeforms
Loading

CRS Bug Bounty PR assessment

  • Rules affected (list rules): 942180
  • 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) : substantial - needs to be validated on real traffic; in case of many FP parts of the regexp may need to be moved to PL3
  • Regular expression quality (inspirational, decent base, needs work, adequate or explain) : adequate
  • Documentation (needs work, adequate or explain) : N/A
  • Tests (none or some or adequate) : some (no negative tests)
     
  • Verdict (Unusable, inspirational, usable, almost perfect or perfect) : usable (have to be tested on false positives on traffic; does not completely fix 2nd issue)

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

@fzipi fzipi changed the title additional sqli auth bypasses fix(sqli): additional sqli auth bypasses Jun 4, 2022
@fzipi fzipi mentioned this pull request Jun 5, 2022
Closed
@dune73 dune73 mentioned this pull request Jul 4, 2022
Closed
@fzipi fzipi mentioned this pull request Aug 1, 2022
Closed
@fzipi fzipi requested a review from lifeforms August 1, 2022 20:37
@fzipi
Copy link
Member
fzipi commented Aug 1, 2022

@lifeforms Can you run this on some traffic to see how it behaves?

@lifeforms
Copy link
Member

Updated rule 942180 and running it now on 3 machines.

@lifeforms lifeforms self-assigned this Aug 3, 2022
@lifeforms
Copy link
Member
lifeforms commented Aug 5, 2022 •
edited
Loading

I looked at the logs. I'm impressed that all my 942180 logs truly point to extensive SQLi attempts.

Here are some:

[id \"942180\"] [msg \"Detects basic SQL authentication bypass attempts 1/3\"] [data \"Matched Data: '=' found within ARGS:type: post') AND EXTRACTVALUE(8857,CONCAT(0x5c,0x71707a6b71,(SELECT (ELT(8857=8857,1))),0x7162707171)) AND ('oiwD'='oiwD\"]

[id \"942180\"] [msg \"Detects basic SQL authentication bypass attempts 1/3\"] [data \"Matched Data: '=' found within ARGS:type: post') AND 6300=CAST((CHR(113)||CHR(112)||CHR(122)||CHR(107)||CHR(113))||(SELECT (CASE WHEN (6300=6300) THEN 1 ELSE 0 END))::text||(CHR(113)||CHR(98)||CHR(112)||CHR(113)||CHR(113)) AS NUMERIC) AND ('XYPW'='XYPW\"]

[id \"942180\"] [msg \"Detects basic SQL authentication bypass attempts 1/3\"] [data \"Matched Data: '=' found within ARGS:type: post') AND 3718 IN (SELECT (CHAR(113) CHAR(112) CHAR(122) CHAR(107) CHAR(113) (SELECT (CASE WHEN (3718=3718) THEN CHAR(49) ELSE CHAR(48) END)) CHAR(113) CHAR(98) CHAR(112) CHAR(113) CHAR(113))) AND ('xIHb'='xIHb\"].

[id \"942180\"] [msg \"Detects basic SQL authentication bypass attempts 1/3\"] [data \"Matched Data: '=' found within ARGS:type: post') AND 8350=(SELECT UPPER(XMLType(CHR(60)||CHR(58)||CHR(113)||CHR(112)||CHR(122)||CHR(107)||CHR(113)||(SELECT (CASE WHEN (8350=8350) THEN 1 ELSE 0 END) FROM DUAL)||CHR(113)||CHR(98)||CHR(112)||CHR(113)||CHR(113)||CHR(62))) FROM DUAL) AND ('CooS'='CooS\"]

[id \"942180\"] [msg \"Detects basic SQL authentication bypass attempts 1/3\"] [data \"Matched Data: '=' found within ARGS:id: 14557002' AND 8952 IN (SELECT (CHAR(113) CHAR(98) CHAR(106) CHAR(120) CHAR(113) (SELECT (CASE WHEN (8952=8952) THEN CHAR(49) ELSE CHAR(48) END)) CHAR(113) CHAR(113) CHAR(112) CHAR(106) CHAR(113))) AND 'yRYr'='yRYr\"]

[id \"942180\"] [msg \"Detects basic SQL authentication bypass attempts 1/3\"] [data \"Matched Data: '=' found within ARGS:notice_id: 220' AND 7388=CAST((CHR(113)||CHR(107)||CHR(118)||CHR(118)||CHR(113))||(SELECT (CASE WHEN (7388=7388) THEN 1 ELSE 0 END))::text||(CHR(113)||CHR(122)||CHR(120)||CHR(113)||CHR(113)) AS NUMERIC) AND 'furx'='furx\"]

[id \"942180\"] [msg \"Detects basic SQL authentication bypass attempts 1/3\"] [data \"Matched Data: '=' found within ARGS:css: public:normalize.css,public:fa.css,public:core.less,public:app.less') AND 7846 IN (SELECT (CHAR(113) CHAR(120) CHAR(106) CHAR(112) CHAR(113) (SELECT (CASE WHEN (7846=7846) THEN CHAR(49) ELSE CHAR(48) END)) CHAR(113) CHAR(107) CHAR(112) CHAR(122) CHAR(113))) AND ('pcDJ'='pcDJ\"]

That's quite some. Very nice that this is all detected by the rule! 🥳

curl "http://localhost/?test=a'='b" is a positive for this rule, so it might be a source of false positives. However, libinjection also triggers on that pattern! So many payloads like this are already blocked - it is not necessarily a new FP (except if you have a high threshold set, which is not recommended).

Note though: I have many exclusions for my hosted apps, so I may be missing some false positives.

I'm tempted to try it out, and if during testing of nightly and the next RC it appears to be unusably trigger happy, we can move it to PL2.

@lifeforms lifeforms merged commit 288d8b2 into coreruleset:v4.0/dev Aug 5, 2022
@dune73
Copy link
Member
dune73 commented Aug 10, 2022

This is not really solving the HTTP Parameter Pollution problem in 5UXE4RK0.

See #2747 for a solution.

@fzipi fzipi mentioned this pull request Sep 5, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lifeforms lifeforms Awaiting requested review from lifeforms

Assignees

@lifeforms lifeforms

Labels
⭐ 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
@rekter0 @dune73 @fzipi @lifeforms @RedXanadu
0