Unified: Add SQL injection query

unified/ql/lib/ext/legacy-swift.model.yml

@@ -111,13 +111,13 @@ extensions:
       - ["", "", false, "sqlite3_bind_text16(_:_:_:_:_:)", "", "", "Argument[2]", "database-store", "manual"]
       - ["", "", false, "sqlite3_bind_text64(_:_:_:_:_:_:)", "", "", "Argument[2]", "database-store", "manual"]
       - ["", "", false, "sqlite3_bind_value(_:_:_:)", "", "", "Argument[2]", "database-store", "manual"]
-      - ["", "", false, "sqlite3_exec(_:_:_:_:_:)", "", "", "Argument[1]", "database-store", "manual"]
-      - ["", "", false, "sqlite3_prepare(_:_:_:_:_:)", "", "", "Argument[1]", "database-store", "manual"]
-      - ["", "", false, "sqlite3_prepare16(_:_:_:_:_:)", "", "", "Argument[1]", "database-store", "manual"]
-      - ["", "", false, "sqlite3_prepare16_v2(_:_:_:_:_:)", "", "", "Argument[1]", "database-store", "manual"]
-      - ["", "", false, "sqlite3_prepare16_v3(_:_:_:_:_:)", "", "", "Argument[1]", "database-store", "manual"]
-      - ["", "", false, "sqlite3_prepare_v2(_:_:_:_:_:)", "", "", "Argument[1]", "database-store", "manual"]
-      - ["", "", false, "sqlite3_prepare_v3(_:_:_:_:_:_:)", "", "", "Argument[1]", "database-store", "manual"]
+      - ["", "", false, "sqlite3_exec(_:_:_:_:_:)", "", "", "Argument[1]", "sql-injection", "manual"]
+      - ["", "", false, "sqlite3_prepare(_:_:_:_:_:)", "", "", "Argument[1]", "sql-injection", "manual"]
+      - ["", "", false, "sqlite3_prepare_v2(_:_:_:_:_:)", "", "", "Argument[1]", "sql-injection", "manual"]
+      - ["", "", false, "sqlite3_prepare_v3(_:_:_:_:_:_:)", "", "", "Argument[1]", "sql-injection", "manual"]
+      - ["", "", false, "sqlite3_prepare16(_:_:_:_:_:)", "", "", "Argument[1]", "sql-injection", "manual"]
+      - ["", "", false, "sqlite3_prepare16_v2(_:_:_:_:_:)", "", "", "Argument[1]", "sql-injection", "manual"]
+      - ["", "", false, "sqlite3_prepare16_v3(_:_:_:_:_:_:)", "", "", "Argument[1]", "sql-injection", "manual"]
       - ["", "", false, "sqlite3_database_file_object(_:)", "", "", "Argument[0]", "path-injection", "manual"]
       - ["", "", false, "sqlite3_filename_database(_:)", "", "", "Argument[0]", "path-injection", "manual"]
       - ["", "", false, "sqlite3_filename_journal(_:)", "", "", "Argument[0]", "path-injection", "manual"]
@@ -230,13 +230,23 @@ extensions:
       - ["", "Connection", true, "keyAndMigrate(_:db:)", "", "", "Argument[0]", "encryption-key", "manual"]
       - ["", "Connection", true, "rekey(_:db:)", "", "", "Argument[0]", "encryption-key", "manual"]
       - ["", "Connection", true, "sqlcipher_export(_:key:)", "", "", "Argument[key:]", "encryption-key", "manual"]
-      - ["", "Connection", true, "execute(_:)", "", "", "Argument[0]", "database-store", "manual"]
-      - ["", "Connection", true, "prepare(_:_:)", "", "", "Argument[0]", "database-store", "manual"]
-      - ["", "Connection", true, "prepare(_:_:)", "", "", "Argument[1]", "database-store", "manual"]
       - ["", "Connection", true, "run(_:_:)", "", "", "Argument[0]", "database-store", "manual"]
       - ["", "Connection", true, "run(_:_:)", "", "", "Argument[1]", "database-store", "manual"]
       - ["", "Connection", true, "scalar(_:_:)", "", "", "Argument[0]", "database-store", "manual"]
       - ["", "Connection", true, "scalar(_:_:)", "", "", "Argument[1]", "database-store", "manual"]
+      - ["", "Connection", true, "execute(_:)", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["", "Connection", true, "prepare(_:)", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["", "Connection", true, "prepare(_:_:)", "", "", "Argument[0]", "sql-injection", "manual"]
+      - ["", "Statement", true, "init(_:_:)", "", "", "Argument[1]", "sql-injection", "manual"]
+      - ["", "SQLRequest", true, "init(stringLiteral:)", "", "", "Argument[stringLiteral:]", "sql-injection", "manual"]
+      - ["", "SQLRequest", true, "init(unicodeScalarLiteral:)", "", "", "Argument[unicodeScalarLiteral:]", "sql-injection", "manual"]
+      - ["", "SQLRequest", true, "init(extendedGraphemeClusterLiteral:)", "", "", "Argument[extendedGraphemeClusterLiteral:]", "sql-injection", "manual"]
+      - ["", "SQLRequest", true, "init(stringInterpolation:)", "", "", "Argument[stringInterpolation:]", "sql-injection", "manual"]
+      - ["", "SQL", true, "init(stringLiteral:)", "", "", "Argument[stringLiteral:]", "sql-injection", "manual"]
+      - ["", "SQL", true, "init(unicodeScalarLiteral:)", "", "", "Argument[unicodeScalarLiteral:]", "sql-injection", "manual"]
+      - ["", "SQL", true, "init(extendedGraphemeClusterLiteral:)", "", "", "Argument[extendedGraphemeClusterLiteral:]", "sql-injection", "manual"]
+      - ["", "SQL", true, "init(stringInterpolation:)", "", "", "Argument[stringInterpolation:]", "sql-injection", "manual"]
+      - ["", "StatementCache", true, "statement(_:)", "", "", "Argument[0]", "sql-injection", "manual"]
       - ["", "Connection", true, "init(_:readonly:)", "", "", "Argument[0]", "path-injection", "manual"]
       - ["", "Connection.Location", true, "uri(_:parameters:)", "", "", "Argument[0]", "path-injection", "manual"]
       - ["", "Database", true, "changePassphrase(_:)", "", "", "Argument[0]", "encryption-key", "manual"]
@@ -341,7 +351,6 @@ extensions:
       - ["", "SQL", true, "init(arguments:sql:)", "", "", "Argument[arguments:]", "database-store", "manual"]
       - ["", "SQLRequest", true, "init(adapter:arguments:cached:sql:)", "", "", "Argument[arguments:]", "database-store", "manual"]
       - ["", "SQLStatementCursor", true, "init(arguments:database:prepFlags:sql:)", "", "", "Argument[arguments:]", "database-store", "manual"]
-      - ["", "Statement", true, "init(_:_:)", "", "", "Argument[1]", "database-store", "manual"]
       - ["", "Statement", true, "scalar(_:)", "", "", "Argument[0]", "database-store", "manual"]
       - ["", "Statement", true, "bind(_:)", "", "", "Argument[0]", "database-store", "manual"]
       - ["", "Statement", true, "execute(arguments:)", "", "", "Argument[arguments:]", "database-store", "manual"]

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

@@ -0,0 +1,38 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+
+<p>
+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.
+</p>
+
+</overview>
+<recommendation>
+
+<p>
+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.
+</p>
+
+</recommendation>
+<example>
+
+<p>In the following examples, an SQL query is prepared using string interpolation to directly include a user-controlled value <code>userControlledString</code> in the query. An attacker could craft <code>userControlledString</code> to change the overall meaning of the SQL query.
+</p>
+
+<sample src="SqlInjectionBad.swift" />
+
+<p>A better way to do this is with a prepared statement, binding <code>userControlledString</code> to a parameter of that statement. An attacker who controls <code>userControlledString</code> now cannot change the overall meaning of the query.
+</p>
+
+<sample src="SqlInjectionGood.swift" />
+
+</example>
+<references>
+
+<li>Wikipedia: <a href="https://en.wikipedia.org/wiki/SQL_injection">SQL injection</a>.</li>
+<li>OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html">SQL Injection Prevention Cheat Sheet</a>.</li>
+
+</references>
+</qhelp>

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

@@ -0,0 +1,76 @@
+/**
+ * @name Database query built from user-controlled sources
+ * @description Building a database query from user-controlled sources is vulnerable to insertion of malicious code by the user.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 8.8
+ * @precision high
+ * @id unified/swift/sql-injection
+ * @tags security
+ *       external/cwe/cwe-089
+ */
+
+import unified
+private import codeql.unified.internal.typeinference.TypeInference as TypeInference
+
+private predicate sqliteSwiftSink(DataFlow::Node node) {
+  // 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
+    target.getName() = ["run", "scalar"] and
+    node.asExpr() = call.getArgument(0).getValue()
+  )
+}
+
+/**
+ * An SQL injection sink that is determined by imprecise methods.
+ */
+private predicate heuristicSink(DataFlow::Node node) {
+  exists(Argument arg |
+    node.asExpr() = arg.getValue() and
+    (
+      // by argument name
+      arg.getName() = "sql"
+      or
+      // by parameter name
+      // TODO: make a helper predicate `Argument.getParameter` for this?
+      exists(CallExpr call, int index, Parameter param |
+        arg = call.getArgument(index) and
+        param = TypeInference::resolveCallTarget(call).getParameter(index) and
+        param.getPattern().(Identifier).getValue() = "sql"
+      )
+    )
+  )
+}
+
+/**
+ * A taint configuration for tainted data that reaches a SQL sink.
+ */
+module SqlInjectionConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { Models::isSource(node, _) }
+
+  predicate isSink(DataFlow::Node node) {
+    Models::isSink(node, "sql-injection")
+    or
+    sqliteSwiftSink(node)
+    or
+    heuristicSink(node)
+  }
+
+  predicate isBarrier(DataFlow::Node node) {
+    // TODO: Port the numeric-type barrier from original Swift's SqlInjectionExtensions.qll.
+    none()
+  }
+
+  predicate observeDiffInformedIncrementalMode() { any() }
+}
+
+module SqlInjectionFlow = TaintTracking::Global<SqlInjectionConfig>;
+
+import SqlInjectionFlow::PathGraph
+
+from SqlInjectionFlow::PathNode sourceNode, SqlInjectionFlow::PathNode sinkNode
+where SqlInjectionFlow::flowPath(sourceNode, sinkNode)
+select sinkNode.getNode(), sourceNode, sinkNode, "This query depends on a $@.",
+  sourceNode.getNode(), "user-provided value"

unified/ql/src/queries/security/CWE-089/SqlInjectionBad.swift

@@ -0,0 +1,12 @@
+// 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

unified/ql/src/queries/security/CWE-089/SqlInjectionGood.swift

@@ -0,0 +1,19 @@
+// 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)
+}