Unified: Add SQL injection query by geoffw0 · Pull Request #22942 · github/codeql

geoffw0 · 2026-10-29T15:46:03Z

Adds a unified/swift/sql-injection query for unified Swift. Copilot did this step-by-step using a skill I'm developing, I did a bit of fixup afterwards, and a couple of TODO comments remain for later follow-up.

Original docs are preserved, tests are preserved and expanded slightly, and in fact this query does very well on the tests.

~~Will see how it does on DCA shortly...~~ DCA LGTM.

Establish a byte-for-byte baseline for the unified Swift port, including query help, examples and the original test expectations. API and reference adaptations follow separately. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copy the original data flow configuration and query-specific models into the query, preserving declarations and provenance before adapting unified interfaces. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Use unified taint tracking, model-backed sources and sinks, and the sql argument-label heuristic. Retain all original cases and mark unsupported detections as missing. Parameter-name heuristics, SQL sink models and numeric barriers remain follow-up work. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add SQLite C API, SQLite.swift and GRDB sinks not already covered by sql argument labels. Cover omitted variadic bindings while retaining safe prepared-statement cases. Leave Connection.run/scalar missing because legacy selectors cannot distinguish safe Statement binding methods. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Match sql parameters using unified call resolution, accounting for positional and external argument names. Cover tainted non-SQL arguments as negative cases and preserve all previous alert paths. All four unified security query tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Use resolved Connection targets until legacy models support receiver checks, avoiding false positives on Statement bindings. Cover omitted and multiple variadic bindings with unsafe SQL and safe parameterized comparisons. Preserve all preceding detections; security and model tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Explain that preparing interpolated SQL does not prevent injection and prefer parameter binding over fragile escaping. Verify both external references, XML parsing and referenced Swift samples. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…ases.

…rget confusion.

…better.

…tabase storage query can just match both patterns, as the Rust version does; any potential SQL injection is input to a database).

github-actions · 2026-10-29T15:46:58Z

QHelp previews:

unified/ql/src/queries/security/CWE-089/SqlInjection.qhelp

Database query built from user-controlled sources

If a database query (such as a SQL query) is built from user-provided data without sufficient sanitization, a user may be able to run malicious database queries. An attacker can craft the part of the query they control to change the overall meaning of the query.

Recommendation

Most database connector libraries offer a way to safely embed untrusted data into a query using query parameters or prepared statements. You should use these features to build queries, rather than string concatenation or similar methods. You can also escape (sanitize) user-controlled strings so that they can be included directly in an SQL command. A library function should be used for escaping, because this approach is only safe if the escaping function is robust against all possible inputs.

Example

In the following examples, an SQL query is prepared using string interpolation to directly include a user-controlled value userControlledString in the query. An attacker could craft userControlledString to change the overall meaning of the SQL query.

// with SQLite.swift

let unsafeQuery = "SELECT * FROM users WHERE username='\(userControlledString)'"

try db.execute(unsafeQuery) // BAD

let stmt = try db.prepare(unsafeQuery) // also BAD
try stmt.run()

// with SQLite3 C API

let result = sqlite3_exec(db, unsafeQuery, nil, nil, nil) // BAD

A better way to do this is with a prepared statement, binding userControlledString to a parameter of that statement. An attacker who controls userControlledString now cannot change the overall meaning of the query.

// with SQLite.swift

let safeQuery = "SELECT * FROM users WHERE username=?"

let stmt = try db.prepare(safeQuery, userControlledString) // GOOD
try stmt.run()

// with sqlite3 C API

var stmt2: OpaquePointer?

if (sqlite3_prepare_v2(db, safeQuery, -1, &stmt2, nil) == SQLITE_OK) {
	if (sqlite3_bind_text(stmt2, 1, userControlledString, -1, SQLITE_TRANSIENT) == SQLITE_OK) { // GOOD
		let result = sqlite3_step(stmt2)

		// ...
	}
	sqlite3_finalize(stmt2)
}

References

Wikipedia: SQL injection.
OWASP: SQL Injection Prevention Cheat Sheet.
Common Weakness Enumeration: CWE-89.

Copilot

🟡 Changes recommended

The SQL injection model misses extension-defined sinks and multi-binding prepare calls, while incorrect argument matching causes missed alerts and spurious matches.

3 open issues

What changed in this PR

Adds unified/swift/sql-injection to the unified Swift analysis, detecting user-controlled data that reaches SQL query arguments.

Changes:

Introduces path-based taint analysis with modeled and heuristic SQL sinks.
Extends Swift models for SQLite and GRDB-related APIs.
Preserves documentation and examples, and ports and expands regression tests.

File	Description
`unified/ql/test/query-tests/security/CWE-089/SqlInjection/sqlite3_c_api.swift`	Tests SQLite C execution, preparation, and safe parameter binding.
`unified/ql/test/query-tests/security/CWE-089/SqlInjection/SQLite.swift`	Tests SQLite.swift query APIs and binding variants.
`unified/ql/test/query-tests/security/CWE-089/SqlInjection/SqlInjection.qlref`	Connects the query to inline-expectation testing.
`unified/ql/test/query-tests/security/CWE-089/SqlInjection/SqlInjection.expected`	Records expected alerts and taint paths.
`unified/ql/test/query-tests/security/CWE-089/SqlInjection/other.swift`	Tests heuristic SQL argument recognition.
`unified/ql/test/query-tests/security/CWE-089/SqlInjection/GRDB.swift`	Tests GRDB raw-SQL APIs and constructors.
`unified/ql/src/queries/security/CWE-089/SqlInjectionGood.swift`	Demonstrates safe parameter binding.
`unified/ql/src/queries/security/CWE-089/SqlInjectionBad.swift`	Demonstrates unsafe SQL interpolation.
`unified/ql/src/queries/security/CWE-089/SqlInjection.ql`	Implements SQL-injection detection and path reporting.
`unified/ql/src/queries/security/CWE-089/SqlInjection.qhelp`	Documents the vulnerability and prevention techniques.
`unified/ql/lib/ext/legacy-swift.model.yml`	Adds SQL-injection sink classifications and corrects a SQLite selector.

_{Review effort: Max (auto)}

+  // TODO: Use MaD once legacy selectors distinguish Connection methods from Statement bindings.
+  exists(CallExpr call, FunctionDeclaration target |
+    target = TypeInference::resolveCallTarget(call) and
+    target.getEnclosingClass().getName() = "Connection" and


+  exists(CallExpr call, FunctionDeclaration target |
+    target = TypeInference::resolveCallTarget(call) and
+    target.getEnclosingClass().getName() = "Connection" and
+    target.getName() = ["run", "scalar"] and


+        arg = call.getArgument(index) and
+        param = TypeInference::resolveCallTarget(call).getParameter(index) and


geoffw0 and others added 13 commits October 28, 2026 11:37

unified: move the new test cases into organization of existing test c…

42158ff

…ases.

unified: Add / edit doc and comments.

c0afa1f

unified: Add a few more test cases around parameter names and call ta…

b1a43f7

…rget confusion.

unified: Clean up heuristicSink predicate.

2bcd3ab

unified: Undo the .qhelp change. I'm not convinced it's meaningfully …

5921954

…better.

unified: Remove database-store sinks that duplicate sql-sinks (the da…

c38ed2a

…tabase storage query can just match both patterns, as the Rust version does; any potential SQL injection is input to a database).

geoffw0 added no-change-note-required This PR does not need a change note Unified labels Oct 29, 2026

github-actions Bot added the documentation label Oct 29, 2026

geoffw0 marked this pull request as ready for review October 30, 2026 18:48

Copilot AI auto review requested due to automatic review settings October 30, 2026 18:48

geoffw0 requested a review from a team as a code owner October 30, 2026 18:48

Copilot started reviewing on behalf of geoffw0 October 30, 2026 18:49

View session

Copilot AI reviewed Oct 30, 2026

View reviewed changes

Navigation Menu

Repository navigation

Unified: Add SQL injection query - #22942

Unified: Add SQL injection query#22942
geoffw0 wants to merge 13 commits into
github:mainfrom
geoffw0:unifiedsqlinjection

geoffw0 commented Oct 29, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 29, 2026

Database query built from user-controlled sources

Recommendation

Example

References

Uh oh!

Copilot AI left a comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		arg = call.getArgument(index) and
		param = TypeInference::resolveCallTarget(call).getParameter(index) and

📋 Server Request & Proxy Logs

⚙️ Pengaturan Proxy Target

Conversation

geoffw0 commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 29, 2026

Database query built from user-controlled sources

Recommendation

Example

References

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

geoffw0 commented Oct 29, 2026 •

edited

Loading