Conversation
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.
…409) * Add server tests for invalid SQL blocking (AIKIDO_BLOCK_INVALID_SQL) * fix
…ntainability. Updated return values in detection functions to use constants from zen_internals for better readability.
![aikido-pr-checks[bot]](https://avatars.githubusercontent.com/in/898896?s=60&v=4){kind=small}
| } | ||
| } | ||
|
|
||
| if result == zen_internals.SQLInjectionTokenizeFailed && blockInvalidSql { |
There was a problem hiding this comment.
Two near-identical InterceptorResult constructions were added; consolidate the repeated result-building to avoid duplicated logic.
Details
✨ AI Reasoning
The change added a new branch that returns an InterceptorResult with the same structure as the existing SQLInjectionDetected branch, differing only by one metadata key (failedToTokenize). Both branches build and return the same type with identical fields (Operation, Kind, Source, PathToPayload, Metadata contains sql and dialect, and Payload). This duplicates substantial, non-trivial logic in the same function and would require parallel updates if the result shape changes.
🔧 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
…. Updated metadata handling in interceptor results to include a flag for failed tokenization, improving clarity in response data.
Clarified that blocking of invalid SQL queries is not enabled by default and can be activated using the `AIKIDO_BLOCK_INVALID_SQL` environment variable. Adjusted wording for improved clarity on the behavior in blocking and detection-only modes.
Changed the default value of the `AIKIDO_BLOCK_INVALID_SQL` environment variable from true to false, reflecting the updated behavior in the handling of SQL queries that fail tokenization. This aligns with recent documentation updates and clarifies the feature's default state.
…ted logic Changed the default value of the BlockInvalidSql field in AikidoConfigData from true to false. Updated the CheckContextForSqlInjection function to reflect this change, ensuring that the SQL injection detection logic aligns with the new default behavior. This update clarifies the handling of invalid SQL queries in the system.
3 participants
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
⚡ Enhancements
🔧 Refactors
More info