Block SQL queries that fail tokenization

.github/workflows/build.yml

@@ -214,7 +214,7 @@ jobs:
           echo $AIKIDO_VERSION
           echo "AIKIDO_VERSION=$AIKIDO_VERSION" >> $GITHUB_ENV
           echo "AIKIDO_LIBZEN=libzen_internals_${{ env.ARCH }}-unknown-linux-gnu.so" >> $GITHUB_ENV
-          echo "AIKIDO_LIBZEN_VERSION=0.1.60" >> $GITHUB_ENV
+          echo "AIKIDO_LIBZEN_VERSION=0.1.61" >> $GITHUB_ENV
 
       - name: Download artifacts (NTS)
         uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4

docs/invalid-sql-queries.md

@@ -0,0 +1,8 @@
+# Blocking invalid SQL queries
+
+Zen blocks SQL queries that it can't tokenize when they contain user input. This prevents attackers from bypassing SQL injection detection with malformed queries. For example, ClickHouse ignores invalid SQL after `;`, and SQLite runs queries before an unclosed `/*` comment.
+
+This is on by default. In blocking mode, these queries are blocked. In detection-only mode, they are reported but still executed.
+
+If you see false positives (legitimate queries being blocked), set the
+`AIKIDO_BLOCK_INVALID_SQL` environment variable to `false`.

lib/php-extension/Environment.cpp

@@ -228,6 +228,7 @@ void LoadEnvironmentFromGetters(const std::vector<EnvGetterFn>& envGetters) {
     AIKIDO_GLOBAL(collect_api_schema) = GetEnvBool(envGetters,"AIKIDO_FEATURE_COLLECT_API_SCHEMA", true);
     AIKIDO_GLOBAL(localhost_allowed_by_default) = GetEnvBool(envGetters, "AIKIDO_LOCALHOST_ALLOWED_BY_DEFAULT", true);
     AIKIDO_GLOBAL(trust_proxy) = GetEnvBool(envGetters, "AIKIDO_TRUST_PROXY", true);
+    AIKIDO_GLOBAL(block_invalid_sql) = GetEnvBool(envGetters, "AIKIDO_BLOCK_INVALID_SQL", true);
     AIKIDO_GLOBAL(disk_logs) = GetEnvBool(envGetters, "AIKIDO_DISK_LOGS", false);
     AIKIDO_GLOBAL(sapi_name) = sapi_module.name;
     AIKIDO_GLOBAL(token) = GetEnvString(envGetters, "AIKIDO_TOKEN", "");

lib/php-extension/RequestProcessor.cpp

@@ -96,6 +96,7 @@ std::string RequestProcessor::GetInitData(const std::string& userProvidedToken)
         {"disk_logs", AIKIDO_GLOBAL(disk_logs)},
         {"localhost_allowed_by_default", AIKIDO_GLOBAL(localhost_allowed_by_default)},
         {"collect_api_schema", AIKIDO_GLOBAL(collect_api_schema)},
+        {"block_invalid_sql", AIKIDO_GLOBAL(block_invalid_sql)},
         {"packages", packages}};
     return NormalizeAndDumpJson(initData);
 }

lib/php-extension/include/php_aikido.h

@@ -34,6 +34,7 @@ bool disk_logs; // When enabled, it writes logs to disk instead of stdout. It's
 bool collect_api_schema;
 bool trust_proxy;
 bool localhost_allowed_by_default;
+bool block_invalid_sql; // Block SQL queries that fail tokenization when user input is present (AIKIDO_BLOCK_INVALID_SQL, default: true)
 bool uses_symfony_http_foundation; // If true, method override is supported using X-HTTP-METHOD-OVERRIDE or _method query param
 unsigned int report_stats_interval_to_agent; // Report once every X requests the collected stats to Agent
 std::chrono::high_resolution_clock::time_point currentRequestStart;

lib/request-processor/aikido_types/config.go

@@ -23,6 +23,7 @@ type AikidoConfigData struct {
 	TrustProxy                bool              `json:"trust_proxy"`                  // default: true
 	LocalhostAllowedByDefault bool              `json:"localhost_allowed_by_default"` // default: true
 	CollectApiSchema          bool              `json:"collect_api_schema"`           // default: true
+	BlockInvalidSql           bool              `json:"block_invalid_sql"`            // default: true
 	DiskLogs                  bool              `json:"disk_logs"`                    // default: false
 	Packages                  map[string]string `json:"packages"`                     // default: {}
 }

lib/request-processor/vulnerabilities/sql-injection/checkContextForSqlInjection.go

@@ -5,6 +5,7 @@ import (
 	"main/helpers"
 	"main/instance"
 	"main/utils"
+	zen_internals "main/vulnerabilities/zen-internals"
 )
 
 /**
@@ -15,26 +16,37 @@ func CheckContextForSqlInjection(instance *instance.RequestProcessorInstance, sq
 	trimmedSql := helpers.TrimInvisible(sql)
 	dialectId := utils.GetSqlDialectFromString(dialect)
 
+	blockInvalidSql := true
+	if server := instance.GetCurrentServer(); server != nil {
+		blockInvalidSql = server.AikidoConfig.BlockInvalidSql
+	}
+
 	for _, source := range context.SOURCES {
 		mapss := source.CacheGet(instance)
 
 		for str, path := range mapss {
 			trimmedInputString := helpers.TrimInvisible(str)
-			if detectSQLInjection(trimmedSql, trimmedInputString, dialectId) {
+			result := detectSQLInjection(trimmedSql, trimmedInputString, dialectId)
+
+			if (result == zen_internals.SQLInjectionDetected) ||
+				(result == zen_internals.SQLInjectionTokenizeFailed && blockInvalidSql) {
+				metadata := map[string]string{
+					"sql":     sql,
+					"dialect": dialect,
+				}
+				if result == zen_internals.SQLInjectionTokenizeFailed {
+					metadata["failedToTokenize"] = "true"
+				}
 				return &utils.InterceptorResult{
 					Operation:     operation,
 					Kind:          utils.Sql_injection,
 					Source:        source.Name,
 					PathToPayload: path,
-					Metadata: map[string]string{
-						"sql":     sql,
-						"dialect": dialect,
-					},
-					Payload: str,
+					Metadata:      metadata,
+					Payload:       str,
 				}
 			}
 		}
 	}
 	return nil
-
 }

lib/request-processor/vulnerabilities/sql-injection/detectSqlInjection.go

@@ -34,12 +34,11 @@ func shouldReturnEarly(query, userInput string) bool {
 	return match
 }
 
-func detectSQLInjection(query string, userInput string, dialect int) bool {
+func detectSQLInjection(query string, userInput string, dialect int) int {
 	if shouldReturnEarly(query, userInput) {
-		return false
+		return zen_internals.SqlInjectionClean
 	}
 
 	// Executing our final check with zen_internals
-	return zen_internals.DetectSQLInjection(query, userInput, dialect) == 1
-
+	return zen_internals.DetectSQLInjection(query, userInput, dialect)
 }

lib/request-processor/vulnerabilities/zen-internals/zen_internals.go

@@ -33,6 +33,13 @@ type ZenInternalsLibrary struct {
 	detectSqlInjection C.detect_sql_injection_func
 }
 
+const (
+	SqlInjectionClean          = 0
+	SQLInjectionDetected       = 1
+	SQLInjectionError          = 2
+	SQLInjectionTokenizeFailed = 3
+)
+
 var zenLib = &ZenInternalsLibrary{}
 
 func Init() bool {
@@ -75,7 +82,7 @@ func DetectSQLInjection(query string, user_input string, dialect int) int {
 	detectFn := zenLib.detectSqlInjection
 
 	if detectFn == nil {
-		return 0
+		return SqlInjectionClean
 	}
 
 	// Convert strings to C strings

tests/server/test_sql_injection_invalid_sql_blocked/env.json

@@ -0,0 +1,6 @@
+{
+    "AIKIDO_BLOCK": "1",
+    "AIKIDO_BLOCK_INVALID_SQL": "1",
+    "AIKIDO_LOCALHOST_ALLOWED_BY_DEFAULT": "0",
+    "AIKIDO_FEATURE_COLLECT_API_SCHEMA": "1"
+}

tests/server/test_sql_injection_invalid_sql_blocked/expect_detection_blocked.json

@@ -0,0 +1,31 @@
+{
+    "type": "detected_attack",
+    "request": {
+        "method": "GET",
+        "source": "php",
+        "route": "/testDetection"
+    },
+    "attack": {
+        "kind": "sql_injection",
+        "operation": "pdo->query",
+        "module": "PDO",
+        "blocked": true,
+        "source": "query",
+        "path": ".id",
+        "stack": "tests/server/test_sql_injection_invalid_sql_blocked/index.php(18): PDO->query()",
+        "payload": "1 /*",
+        "metadata": {
+            "dialect": "sqlite",
+            "sql": "SELECT * FROM cats WHERE id = 1 /*",
+            "failedToTokenize": "true"
+        },
+        "user": {
+            "id": "12345",
+            "name": "Test User"
+        }
+    },
+    "agent": {
+        "dryMode": false,
+        "library": "firewall-php"
+    }
+}

tests/server/test_sql_injection_invalid_sql_blocked/index.php

@@ -0,0 +1,24 @@
+<?php
+
+\aikido\set_user("12345", "Test User");
+
+try {
+    $pdo = new PDO('sqlite::memory:');
+    $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
+
+    $pdo->exec("CREATE TABLE IF NOT EXISTS cats (
+        id INTEGER PRIMARY KEY AUTOINCREMENT,
+        name TEXT NOT NULL,
+        age INTEGER NOT NULL
+    )");
+
+    $pdo->exec("INSERT INTO cats (name, age) VALUES ('n', 1)");
+
+    $id = $_GET['id'];
+    $pdo->query("SELECT * FROM cats WHERE id = $id");
+
+    echo "Query executed!";
+
+} catch (PDOException $e) {
+    echo "Error: " . $e->getMessage();
+}

tests/server/test_sql_injection_invalid_sql_blocked/start_config.json

@@ -0,0 +1,10 @@
+{
+    "success": true,
+    "serviceId": 1,
+    "heartbeatIntervalInMS": 600000,
+    "endpoints": [],
+    "blockedUserIds": [],
+    "allowedIPAddresses": [],
+    "receivedAnyStats": true,
+    "block": true
+}

tests/server/test_sql_injection_invalid_sql_blocked/test.py

@@ -0,0 +1,24 @@
+from testlib import *
+
+"""
+Invalid SQL (failed tokenization) with user input is blocked when AIKIDO_BLOCK=1 and
+AIKIDO_BLOCK_INVALID_SQL=1. Uses PDO SQLite and an unclosed block comment in the query.
+"""
+
+
+def run_test():
+    response = php_server_get("/testDetection?id=1+%2F*")
+    assert_response_code_is(response, 500)
+    assert_response_body_contains(response, "")
+
+    mock_server_wait_for_new_events(5)
+
+    events = mock_server_get_events()
+    assert_events_length_is(events, 2)
+    assert_started_event_is_valid(events[0])
+    assert_event_contains_subset_file(events[1], "expect_detection_blocked.json")
+
+
+if __name__ == "__main__":
+    load_test_args()
+    run_test()

tests/server/test_sql_injection_invalid_sql_not_blocked/env.json

@@ -0,0 +1,6 @@
+{
+    "AIKIDO_BLOCK": "1",
+    "AIKIDO_BLOCK_INVALID_SQL": "0",
+    "AIKIDO_LOCALHOST_ALLOWED_BY_DEFAULT": "0",
+    "AIKIDO_FEATURE_COLLECT_API_SCHEMA": "1"
+}

tests/server/test_sql_injection_invalid_sql_not_blocked/index.php

@@ -0,0 +1,24 @@
+<?php
+
+\aikido\set_user("12345", "Test User");
+
+try {
+    $pdo = new PDO('sqlite::memory:');
+    $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
+
+    $pdo->exec("CREATE TABLE IF NOT EXISTS cats (
+        id INTEGER PRIMARY KEY AUTOINCREMENT,
+        name TEXT NOT NULL,
+        age INTEGER NOT NULL
+    )");
+
+    $pdo->exec("INSERT INTO cats (name, age) VALUES ('n', 1)");
+
+    $id = $_GET['id'];
+    $pdo->query("SELECT * FROM cats WHERE id = $id");
+
+    echo "Query executed!";
+
+} catch (PDOException $e) {
+    echo "Error: " . $e->getMessage();
+}

tests/server/test_sql_injection_invalid_sql_not_blocked/start_config.json

@@ -0,0 +1,10 @@
+{
+    "success": true,
+    "serviceId": 1,
+    "heartbeatIntervalInMS": 600000,
+    "endpoints": [],
+    "blockedUserIds": [],
+    "allowedIPAddresses": [],
+    "receivedAnyStats": true,
+    "block": true
+}

tests/server/test_sql_injection_invalid_sql_not_blocked/test.py

@@ -0,0 +1,19 @@
+import time
+from testlib import *
+
+"""
+With AIKIDO_BLOCK=1 but AIKIDO_BLOCK_INVALID_SQL=0, queries that only fail tokenization
+(result 3) are not blocked and no attack event is reported.
+"""
+
+
+def run_test():
+    response = php_server_get("/testDetection?id=1+%2F*")
+    assert_response_code_is(response, 200)
+    assert_response_body_contains(response, "Error: SQLSTATE[HY000]")
+    assert_events_length_is(mock_server_get_events(), 1)  # No attack event is reported.
+    assert_started_event_is_valid(mock_server_get_events()[0])
+
+if __name__ == "__main__":
+    load_test_args()
+    run_test()