Skip to content

Block SQL queries that fail tokenization#599

Open
hansott wants to merge 4 commits intomainfrom
block-invalid-sql-queries
Open

Block SQL queries that fail tokenization#599
hansott wants to merge 4 commits intomainfrom
block-invalid-sql-queries

Conversation

@hansott
Copy link
Copy Markdown
Member

@hansott hansott commented Mar 27, 2026

Our SQL injection detection tokenizes queries to check them. If the tokenizer can't parse a query, we skip it and the query goes through. Some databases still execute partially valid queries though: ClickHouse ignores junk after ; and SQLite runs everything before an unclosed /*.

Now when user input shows up in a query we can't tokenize, we treat it as an attack. On by default, opt out with AIKIDO_BLOCK_INVALID_SQL=false.

Summary by Aikido

Security Issues: 0 🔍 Quality Issues: 1 Resolved Issues: 0

🚀 New Features

  • Blocked queries that failed tokenizer when user input was present by default

⚡ Enhancements

  • Converted detect_sql_injection to return integer status codes instead of booleans
  • Added env var and updated context handling to report tokenization failures

More info

Our SQL injection detection tokenizes queries to check them. If the
tokenizer can't parse a query, we skip it and the query goes through.
Some databases still execute partially valid queries though: ClickHouse
ignores junk after ; and SQLite runs everything before an unclosed /*.

Now when user input shows up in a query we can't tokenize, we treat
it as an attack. On by default, opt out with AIKIDO_BLOCK_INVALID_SQL=false.
"payload": user_input,
}

if result == 3 and should_block_invalid_sql_queries():
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Two branches build and return nearly identical SQL injection result dicts (operation, kind, source, pathToPayload, metadata.sql, metadata.dialect, payload). Consolidate into a single helper to avoid duplicated response construction.

Details

✨ AI Reasoning
​The new code adds two conditional branches that both construct and return a very similar dictionary describing a detected SQL injection. Both branches set the same keys: operation, kind, source, pathToPayload, metadata with sql and dialect, and payload. The second branch adds only one extra metadata field failedToTokenize. This repeats nearly identical business-logic for building the result object in two places, increasing maintenance burden: any future change to the shape of the result would need to be applied in both branches and could lead to inconsistent responses.

🔧 How do I fix it?
Delete extra code. Extract repeated code sequences into reusable functions or methods. Use loops or data structures to eliminate repetitive patterns.

Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

hansott added 3 commits March 27, 2026 16:26
Backslash escapes are MySQL-specific, so the unterminated string
caused by a trailing backslash only fails tokenization in MySQL.
The test helpers now check specific return codes (1 for injection,
3 for failed tokenization, 0 for no injection) instead of boolean
equality. Invalid SQL tests use a dedicated is_invalid_sql helper.
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.

1 participant