Skip to content
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.

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

SQL Injection (SQLi) Detection Operator #284

Merged
merged 71 commits into from
May 7, 2024
Merged

SQL Injection (SQLi) Detection Operator #284

merged 71 commits into from
May 7, 2024

Conversation

Anilm3
Copy link
Collaborator

@Anilm3 Anilm3 commented Apr 22, 2024

This PR introduces support for a new operator sqli_detector which can be used for detecting SQL injections given a set of request parameters, an SQL statement and a database dialect. The current set of dialects supported is:

  • generic: which provides a simple tokenizer which should work with most database dialects to a certain degree, given differences in constants and identifiers.
  • mysql
  • sqlite
  • pgsql

Although more dialects will be supported in the future. Currently the tokenizers are manually written with the objective of extracting the most relevant tokens, they are by no means complete tokenizers. These also distinguish reserved keywords (sql_token_type::command) from identifiers (sql_token_type::identifier) to aid in future improvements to the heuristics, however not all reserved keywords have been included in this distinction.

Events generated from this operator have two obfuscation phases, the first one is performed by replacing all constants, strings and numeric values with a ?. The second obfuscation step is performed using the user-provided obfuscation regular expressions.

Some work still remains which will be done in a future PR:

  • sqli_detector fuzzer.
  • Expanding the tokenizer fuzzer corpus.
  • Benchmark scenario.

To use this new operator,a rule such as the following could be used:

  - id: rsp-930-003
    name: SQLi Exploit detection
    tags:
      type: sqli
      category: exploit_detection
      module: rasp
    conditions:
      - parameters:
          resource:
            - address: server.db.statement
          params:
            - address: server.request.query
            - address: server.request.body
            - address: server.request.path_params
            - address: grpc.server.request.message
            - address: graphql.server.all_resolvers
            - address: graphql.server.resolver
          db_type:
            - address: server.db.system
        operator: sqli_detector

And an input that would result in a match given the above rule could look as follows:

{
    "server.request.query": " ' OR 1=1; --", 
    "server.db.statement": "SELECT * FROM users WHERE id ='' OR 1=1; -- ';",
    "server.db.system": "mysql"
}

@codecov-commenter
Copy link

codecov-commenter commented Apr 22, 2024

Codecov Report

Attention: Patch coverage is 88.04143% with 127 lines in your changes are missing coverage. Please review.

Project coverage is 83.95%. Comparing base (48c6909) to head (133f14d).

Files Patch % Lines
src/condition/sqli_detector.cpp 83.92% 10 Missing and 26 partials ⚠️
src/tokenizer/pgsql.cpp 87.36% 6 Missing and 17 partials ⚠️
src/tokenizer/sqlite.cpp 85.10% 10 Missing and 11 partials ⚠️
src/tokenizer/mysql.cpp 89.77% 6 Missing and 12 partials ⚠️
src/tokenizer/generic_sql.cpp 90.99% 3 Missing and 7 partials ⚠️
src/tokenizer/sql_base.cpp 94.35% 7 Missing and 3 partials ⚠️
src/tokenizer/sql_base.hpp 74.19% 4 Missing and 4 partials ⚠️
src/utils.hpp 91.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #284      +/-   ##
==========================================
+ Coverage   83.05%   83.95%   +0.90%     
==========================================
  Files         119      127       +8     
  Lines        4744     5804    +1060     
  Branches     2249     2793     +544     
==========================================
+ Hits         3940     4873     +933     
- Misses        307      353      +46     
- Partials      497      578      +81     
Flag Coverage Δ
waf_test 83.95% <88.04%> (+0.90%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pr-commenter
Copy link

pr-commenter bot commented Apr 22, 2024

Benchmarks

Benchmark execution time: 2024-05-07 14:34:09

Comparing candidate commit 133f14d in PR branch anilm3/sqli with baseline commit 48c6909 in branch master.

Found 2 performance improvements and 4 performance regressions! Performance is the same for 13 metrics, 0 unstable metrics.

scenario:is_xss_matcher.random

  • 🟩 execution_time [-4.421ms; -4.415ms] or [-6.084%; -6.076%]

scenario:phrase_match_matcher.enforce_word_boundary.random

  • 🟥 execution_time [+393.305µs; +395.184µs] or [+6.618%; +6.649%]

scenario:phrase_match_matcher.random

  • 🟩 execution_time [-534.144µs; -530.969µs] or [-8.314%; -8.265%]

scenario:regex_match_matcher.case_insensitive_flag.random

  • 🟥 execution_time [+626.463µs; +628.244µs] or [+14.895%; +14.938%]

scenario:regex_match_matcher.case_insensitive_option.random

  • 🟥 execution_time [+624.646µs; +626.079µs] or [+14.847%; +14.881%]

scenario:regex_match_matcher.lowercase_transformer.random

  • 🟥 execution_time [+646.651µs; +648.405µs] or [+11.424%; +11.455%]

@Anilm3 Anilm3 marked this pull request as ready for review April 25, 2024 10:45
@Anilm3 Anilm3 requested a review from a team as a code owner April 25, 2024 10:45
// The first character is / so it can be a comment or a binary operator
sql_token token;
token.index = index();
if (advance() && peek() == '*') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe for simplicity advance() && peek() could be behind a function called next

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I get your point, although the next() function exists and it's used as a lookahead(1) without advancing the cursor.

Copy link
Collaborator

@estringana estringana left a comment

Choose a reason for hiding this comment

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

Just a few comment but not critical ones

src/condition/sqli_detector.cpp Outdated Show resolved Hide resolved
src/condition/sqli_detector.cpp Outdated Show resolved Hide resolved
src/condition/sqli_detector.cpp Outdated Show resolved Hide resolved
src/condition/sqli_detector.cpp Outdated Show resolved Hide resolved
src/condition/sqli_detector.cpp Outdated Show resolved Hide resolved
src/tokenizer/generic_sql.cpp Outdated Show resolved Hide resolved
src/tokenizer/generic_sql.cpp Outdated Show resolved Hide resolved
src/tokenizer/generic_sql.cpp Show resolved Hide resolved
src/tokenizer/generic_sql.cpp Show resolved Hide resolved
src/tokenizer/sql_base.cpp Outdated Show resolved Hide resolved
@Anilm3 Anilm3 requested a review from cataphract May 5, 2024 15:40
@Anilm3 Anilm3 merged commit 105e8cc into master May 7, 2024
39 checks passed
@Anilm3 Anilm3 deleted the anilm3/sqli branch May 7, 2024 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants