Swift: Modernize.

swift/ql/lib/codeql/swift/security/SqlInjectionExtensions.qll

@@ -8,15 +8,27 @@ import swift
 import codeql.swift.dataflow.DataFlow
 
 /**
- * A `DataFlow::Node` that is a sink for a SQL string to be executed.
+ * A dataflow sink for SQL injection vulnerabilities.
  */
-abstract class SqlSink extends DataFlow::Node { }
+abstract class SqlInjectionSink extends DataFlow::Node { }
 
 /**
- * A sink for the sqlite3 C API.
+ * A sanitizer for SQL injection vulnerabilities.
  */
-class CApiSqlSink extends SqlSink {
-  CApiSqlSink() {
+abstract class SqlInjectionSanitizer extends DataFlow::Node { }
+
+/**
+ * A unit class for adding additional taint steps.
+ */
+class SqlInjectionAdditionalTaintStep extends Unit {
+  abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
+}
+
+/**
+ * A default SQL injection sink for the sqlite3 C API.
+ */
+class CApiDefaultSqlInjectionSink extends SqlInjectionSink {
+  CApiDefaultSqlInjectionSink() {
     // `sqlite3_exec` and variants of `sqlite3_prepare`.
     exists(CallExpr call |
       call.getStaticTarget()
@@ -33,10 +45,10 @@ class CApiSqlSink extends SqlSink {
 }
 
 /**
- * A sink for the SQLite.swift library.
+ * A default SQL injection sink for the `SQLite.swift` library.
  */
-class SQLiteSwiftSqlSink extends SqlSink {
-  SQLiteSwiftSqlSink() {
+class SQLiteSwiftDefaultSqlInjectionSink extends SqlInjectionSink {
+  SQLiteSwiftDefaultSqlInjectionSink() {
     // Variants of `Connection.execute`, `connection.prepare` and `connection.scalar`.
     exists(CallExpr call |
       call.getStaticTarget()
@@ -54,9 +66,11 @@ class SQLiteSwiftSqlSink extends SqlSink {
   }
 }
 
-/** A sink for the GRDB library. */
-class GrdbSqlSink extends SqlSink {
-  GrdbSqlSink() {
+/**
+ * A default SQL injection sink for the GRDB library.
+ */
+class GrdbDefaultSqlInjectionSink extends SqlInjectionSink {
+  GrdbDefaultSqlInjectionSink() {
     exists(CallExpr call, MethodDecl method |
       call.getStaticTarget() = method and
       call.getArgument(0).getExpr() = this.asExpr()

swift/ql/lib/codeql/swift/security/SqlInjectionQuery.qll

@@ -18,5 +18,13 @@ class SqlInjectionConfig extends TaintTracking::Configuration {
 
   override predicate isSource(DataFlow::Node node) { node instanceof FlowSource }
 
-  override predicate isSink(DataFlow::Node node) { node instanceof SqlSink }
+  override predicate isSink(DataFlow::Node node) { node instanceof SqlInjectionSink }
+
+  override predicate isSanitizer(DataFlow::Node sanitizer) {
+    sanitizer instanceof SqlInjectionSanitizer
+  }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+    any(SqlInjectionAdditionalTaintStep s).step(nodeFrom, nodeTo)
+  }
 }

swift/ql/lib/codeql/swift/security/UnsafeJsEvalExtensions.qll

@@ -8,17 +8,27 @@ import codeql.swift.dataflow.DataFlow
 import codeql.swift.dataflow.FlowSources
 
 /**
- * A source of untrusted, user-controlled data.
+ * A dataflow sink for javascript evaluation vulnerabilities.
  */
-class Source = FlowSource;
+abstract class UnsafeJsEvalSink extends DataFlow::Node { }
 
 /**
- * A sink that evaluates a string of JavaScript code.
+ * A sanitizer for javascript evaluation vulnerabilities.
  */
-abstract class Sink extends DataFlow::Node { }
+abstract class UnsafeJsEvalSanitizer extends DataFlow::Node { }
 
-class WKWebView extends Sink {
-  WKWebView() {
+/**
+ * A unit class for adding additional taint steps.
+ */
+class UnsafeJsEvalAdditionalTaintStep extends Unit {
+  abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
+}
+
+/**
+ * A default SQL injection sink for the `WKWebView` interface.
+ */
+class WKWebViewDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
+  WKWebViewDefaultUnsafeJsEvalSink() {
     any(CallExpr ce |
       ce.getStaticTarget()
           .(MethodDecl)
@@ -34,8 +44,11 @@ class WKWebView extends Sink {
   }
 }
 
-class WKUserContentController extends Sink {
-  WKUserContentController() {
+/**
+ * A default SQL injection sink for the `WKUserContentController` interface.
+ */
+class WKUserContentControllerDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
+  WKUserContentControllerDefaultUnsafeJsEvalSink() {
     any(CallExpr ce |
       ce.getStaticTarget()
           .(MethodDecl)
@@ -44,8 +57,11 @@ class WKUserContentController extends Sink {
   }
 }
 
-class UIWebView extends Sink {
-  UIWebView() {
+/**
+ * A default SQL injection sink for the `UIWebView` and `WebView` interfaces.
+ */
+class UIWebViewDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
+  UIWebViewDefaultUnsafeJsEvalSink() {
     any(CallExpr ce |
       ce.getStaticTarget()
           .(MethodDecl)
@@ -54,8 +70,11 @@ class UIWebView extends Sink {
   }
 }
 
-class JSContext extends Sink {
-  JSContext() {
+/**
+ * A default SQL injection sink for the `JSContext` interface.
+ */
+class JSContextDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
+  JSContextDefaultUnsafeJsEvalSink() {
     any(CallExpr ce |
       ce.getStaticTarget()
           .(MethodDecl)
@@ -64,10 +83,61 @@ class JSContext extends Sink {
   }
 }
 
-class JSEvaluateScript extends Sink {
-  JSEvaluateScript() {
+/**
+ * A default SQL injection sink for the `JSEvaluateScript` function.
+ */
+class JSEvaluateScriptDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
+  JSEvaluateScriptDefaultUnsafeJsEvalSink() {
     any(CallExpr ce |
       ce.getStaticTarget().(FreeFunctionDecl).hasName("JSEvaluateScript(_:_:_:_:_:_:)")
     ).getArgument(1).getExpr() = this.asExpr()
   }
 }
+
+/**
+ * A default SQL injection sanitrizer.
+ */
+class DefaultUnsafeJsEvalAdditionalTaintStep extends UnsafeJsEvalAdditionalTaintStep {
+  override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+    exists(Argument arg |
+      arg =
+        any(CallExpr ce |
+          ce.getStaticTarget().(MethodDecl).hasQualifiedName("String", "init(decoding:as:)")
+        ).getArgument(0)
+      or
+      arg =
+        any(CallExpr ce |
+          ce.getStaticTarget()
+              .(FreeFunctionDecl)
+              .hasName([
+                  "JSStringCreateWithUTF8CString(_:)", "JSStringCreateWithCharacters(_:_:)",
+                  "JSStringRetain(_:)"
+                ])
+        ).getArgument(0)
+    |
+      nodeFrom.asExpr() = arg.getExpr() and
+      nodeTo.asExpr() = arg.getApplyExpr()
+    )
+    or
+    exists(CallExpr ce, Expr self, AbstractClosureExpr closure |
+      ce.getStaticTarget()
+          .getName()
+          .matches(["withContiguousStorageIfAvailable(%)", "withUnsafeBufferPointer(%)"]) and
+      self = ce.getQualifier() and
+      ce.getArgument(0).getExpr() = closure
+    |
+      nodeFrom.asExpr() = self and
+      nodeTo.(DataFlow::ParameterNode).getParameter() = closure.getParam(0)
+    )
+    or
+    exists(MemberRefExpr e, Expr self, VarDecl member |
+      self.getType().getName().matches(["Unsafe%Buffer%", "Unsafe%Pointer%"]) and
+      member.getName() = "baseAddress"
+    |
+      e.getBase() = self and
+      e.getMember() = member and
+      nodeFrom.asExpr() = self and
+      nodeTo.asExpr() = e
+    )
+  }
+}

swift/ql/lib/codeql/swift/security/UnsafeJsEvalQuery.qll

@@ -15,51 +15,15 @@ import codeql.swift.security.UnsafeJsEvalExtensions
 class UnsafeJsEvalConfig extends TaintTracking::Configuration {
   UnsafeJsEvalConfig() { this = "UnsafeJsEvalConfig" }
 
-  override predicate isSource(DataFlow::Node node) { node instanceof Source }
+  override predicate isSource(DataFlow::Node node) { node instanceof FlowSource }
 
-  override predicate isSink(DataFlow::Node node) { node instanceof Sink }
+  override predicate isSink(DataFlow::Node node) { node instanceof UnsafeJsEvalSink }
+
+  override predicate isSanitizer(DataFlow::Node sanitizer) {
+    sanitizer instanceof UnsafeJsEvalSanitizer
+  }
 
-  // TODO: convert to new taint flow models
   override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
-    exists(Argument arg |
-      arg =
-        any(CallExpr ce |
-          ce.getStaticTarget().(MethodDecl).hasQualifiedName("String", "init(decoding:as:)")
-        ).getArgument(0)
-      or
-      arg =
-        any(CallExpr ce |
-          ce.getStaticTarget()
-              .(FreeFunctionDecl)
-              .hasName([
-                  "JSStringCreateWithUTF8CString(_:)", "JSStringCreateWithCharacters(_:_:)",
-                  "JSStringRetain(_:)"
-                ])
-        ).getArgument(0)
-    |
-      nodeFrom.asExpr() = arg.getExpr() and
-      nodeTo.asExpr() = arg.getApplyExpr()
-    )
-    or
-    exists(CallExpr ce, Expr self, AbstractClosureExpr closure |
-      ce.getStaticTarget()
-          .getName()
-          .matches(["withContiguousStorageIfAvailable(%)", "withUnsafeBufferPointer(%)"]) and
-      self = ce.getQualifier() and
-      ce.getArgument(0).getExpr() = closure
-    |
-      nodeFrom.asExpr() = self and
-      nodeTo.(DataFlow::ParameterNode).getParameter() = closure.getParam(0)
-    )
-    or
-    exists(MemberRefExpr e, Expr self, VarDecl member |
-      self.getType().getName().matches(["Unsafe%Buffer%", "Unsafe%Pointer%"]) and
-      member.getName() = "baseAddress"
-    |
-      e.getBase() = self and
-      e.getMember() = member and
-      nodeFrom.asExpr() = self and
-      nodeTo.asExpr() = e
-    )
+    any(UnsafeJsEvalAdditionalTaintStep s).step(nodeFrom, nodeTo)
   }
 }

swift/ql/lib/codeql/swift/security/UnsafeWebViewFetchExtensions.qll

@@ -7,13 +7,36 @@ import swift
 import codeql.swift.dataflow.DataFlow
 
 /**
- * A sink that is a candidate result for this query, such as certain arguments
+ * A dataflow sink for unsafe webview fetch vulnerabilities.
+ */
+abstract class UnsafeWebViewFetchSink extends DataFlow::Node {
+  /**
+   * Gets the `baseURL` argument associated with this sink (if any). These arguments affect
+   * the way sinks are reported and are also sinks themselves.
+   */
+  Expr getBaseUrl() { none() }
+}
+
+/**
+ * A sanitizer for unsafe webview fetch vulnerabilities.
+ */
+abstract class UnsafeWebViewFetchSanitizer extends DataFlow::Node { }
+
+/**
+ * A unit class for adding additional taint steps.
+ */
+class UnsafeWebViewFetchAdditionalTaintStep extends Unit {
+  abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
+}
+
+/**
+ * A default uncontrolled format string sink, such as certain arguments
  * to `UIWebView.loadHTMLString`.
  */
-class Sink extends DataFlow::Node {
+class DefaultUnsafeWebViewFetchSink extends UnsafeWebViewFetchSink {
   Expr baseUrl;
 
-  Sink() {
+  DefaultUnsafeWebViewFetchSink() {
     exists(
       MethodDecl funcDecl, CallExpr call, string className, string funcName, int arg, int baseArg
     |
@@ -38,16 +61,13 @@ class Sink extends DataFlow::Node {
         baseArg = 3
       ) and
       call.getStaticTarget() = funcDecl and
-      // match up `funcName`, `paramName`, `arg`, `node`.
+      // match up `className`, `funcName`.
       funcDecl.hasQualifiedName(className, funcName) and
-      call.getArgument(arg).getExpr() = this.asExpr() and
-      // match up `baseURLArg`
-      call.getArgument(baseArg).getExpr() = baseUrl
+      // match up `this`, `baseURL`
+      this.asExpr() = call.getArgument(arg).getExpr() and // URL
+      baseUrl = call.getArgument(baseArg).getExpr() // baseURL
     )
   }
 
-  /**
-   * Gets the `baseURL` argument associated with this sink.
-   */
-  Expr getBaseUrl() { result = baseUrl }
+  override Expr getBaseUrl() { result = baseUrl }
 }

swift/ql/lib/codeql/swift/security/UnsafeWebViewFetchQuery.qll

@@ -19,7 +19,17 @@ class UnsafeWebViewFetchConfig extends TaintTracking::Configuration {
   override predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
 
   override predicate isSink(DataFlow::Node node) {
-    node instanceof Sink or
-    node.asExpr() = any(Sink s).getBaseUrl()
+    exists(UnsafeWebViewFetchSink sink |
+      node = sink or
+      node.asExpr() = sink.getBaseUrl()
+    )
+  }
+
+  override predicate isSanitizer(DataFlow::Node sanitizer) {
+    sanitizer instanceof UnsafeWebViewFetchSanitizer
+  }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+    any(UnsafeWebViewFetchAdditionalTaintStep s).step(nodeFrom, nodeTo)
   }
 }

swift/ql/src/queries/Security/CWE-079/UnsafeWebViewFetch.ql

@@ -19,16 +19,20 @@ import DataFlow::PathGraph
 
 from
   UnsafeWebViewFetchConfig config, DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode,
-  Sink sink, string message
+  UnsafeWebViewFetchSink sink, string message
 where
   config.hasFlowPath(sourceNode, sinkNode) and
   sink = sinkNode.getNode() and
   (
+    // no base URL
+    not exists(sink.getBaseUrl()) and
+    message = "Tainted data is used in a WebView fetch."
+    or
     // base URL is nil
     sink.getBaseUrl() instanceof NilLiteralExpr and
     message = "Tainted data is used in a WebView fetch without restricting the base URL."
     or
-    // base URL is tainted
+    // base URL is also tainted
     config.hasFlowToExpr(sink.getBaseUrl()) and
     message = "Tainted data is used in a WebView fetch with a tainted base URL."
   )

swift/ql/src/queries/Security/CWE-094/UnsafeJsEval.ql

@@ -18,7 +18,7 @@ import codeql.swift.security.UnsafeJsEvalQuery
 import DataFlow::PathGraph
 
 from
-  UnsafeJsEvalConfig config, DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode, Sink sink
+  UnsafeJsEvalConfig config, DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode, UnsafeJsEvalSink sink
 where
   config.hasFlowPath(sourceNode, sinkNode) and
   sink = sinkNode.getNode()