Swift: Unsafe JS Eval Query

.vscode/settings.json

@@ -1,3 +1,5 @@
 {
-    "omnisharp.autoStart": false
+  "omnisharp.autoStart": false,
+  "cmake.sourceDirectory": "${workspaceFolder}/swift",
+  "cmake.buildDirectory": "${workspaceFolder}/bazel-cmake-build"
 }

swift/CMakeLists.txt

@@ -9,6 +9,10 @@ set(CMAKE_CXX_EXTENSIONS OFF)
 set(CMAKE_C_COMPILER clang)
 set(CMAKE_CXX_COMPILER clang++)
 
+if(APPLE)
+    set(CMAKE_OSX_ARCHITECTURES x86_64) # temporary until we can build a Universal Binary
+endif()
+
 project(codeql)
 
 include(../misc/bazel/cmake/setup.cmake)

swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImpl.qll

@@ -1015,6 +1015,14 @@ module Decls {
 
 module Exprs {
   module AssignExprs {
+    /**
+     * The control-flow of a `DiscardAssignmentExpr`, which represents the
+     * `_` leaf expression that may appear on the left-hand side of an `AssignExpr`.
+     */
+    private class DiscardAssignmentExprTree extends AstLeafTree {
+      override DiscardAssignmentExpr ast;
+    }
+
     /**
      * The control-flow of an assignment operation.
      *

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

@@ -0,0 +1,32 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Evaluating JavaScript that contains a substring from a remote origin may lead to remote code execution. Code written by an attacker can execute unauthorized actions, including exfiltration of local data through a third party web service.</p>
+
+</overview>
+<recommendation>
+
+<p>When loading JavaScript into a web view, evaluate only known, locally-defined source code. If part of the input comes from a remote source, do not inject it into the JavaScript code to be evaluated. Instead, send it to the web view as data using an API such as <code>WKWebView.callAsyncJavaScript</code> with the <code>arguments</code> dictionary to pass remote data objects.</p>
+
+</recommendation>
+<example>
+
+<p>In the following (bad) example, a call to <code>WKWebView.evaluateJavaScript</code> evaluates JavaScript source code that is tainted with remote data, potentially introducing a code injection vulnerability.</p>
+
+<sample src="UnsafeJsEvalBad.swift" />
+
+<p>In the following (good) example, we sanitize the remote data by passing it using the <code>arguments</code> dictionary of <code>WKWebView.callAsyncJavaScript</code>. This ensures that untrusted data cannot be evaluated as JavaScript source code.</p>
+
+<sample src="UnsafeJsEvalGood.swift" />
+
+</example>
+<references>
+
+<li>
+  Apple Developer Documentation: <a href="https://developer.apple.com/documentation/webkit/wkwebview/3824703-callasyncjavascript">WKWebView.callAsyncJavaScript(_:arguments:in:contentWorld:)</a>
+</li>
+
+</references>
+</qhelp>

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

@@ -0,0 +1,165 @@
+/**
+ * @name JavaScript Injection
+ * @description Evaluating JavaScript code containing a substring from a remote source may lead to remote code execution.
+ * @kind path-problem
+ * @problem.severity warning
+ * @security-severity 9.3
+ * @precision high
+ * @id swift/unsafe-js-eval
+ * @tags security
+ *       external/cwe/cwe-094
+ *       external/cwe/cwe-095
+ *       external/cwe/cwe-749
+ */
+
+import swift
+import codeql.swift.dataflow.DataFlow
+import codeql.swift.dataflow.TaintTracking
+import codeql.swift.dataflow.FlowSources
+import DataFlow::PathGraph
+
+/**
+ * A source of untrusted, user-controlled data.
+ * TODO: Extend to more (non-remote) sources in the future.
+ */
+class Source = RemoteFlowSource;
+
+/**
+ * A sink that evaluates a string of JavaScript code.
+ */
+abstract class Sink extends DataFlow::Node { }
+
+class WKWebView extends Sink {
+  WKWebView() {
+    any(CallExpr ce |
+      ce.getStaticTarget()
+          .(MethodDecl)
+          .hasQualifiedName("WKWebView",
+            [
+              "evaluateJavaScript(_:)", "evaluateJavaScript(_:completionHandler:)",
+              "evaluateJavaScript(_:in:in:completionHandler:)",
+              "evaluateJavaScript(_:in:contentWorld:)",
+              "callAsyncJavaScript(_:arguments:in:in:completionHandler:)",
+              "callAsyncJavaScript(_:arguments:in:contentWorld:)"
+            ])
+    ).getArgument(0).getExpr() = this.asExpr()
+  }
+}
+
+class WKUserContentController extends Sink {
+  WKUserContentController() {
+    any(CallExpr ce |
+      ce.getStaticTarget()
+          .(MethodDecl)
+          .hasQualifiedName("WKUserContentController", "addUserScript(_:)")
+    ).getArgument(0).getExpr() = this.asExpr()
+  }
+}
+
+class UIWebView extends Sink {
+  UIWebView() {
+    any(CallExpr ce |
+      ce.getStaticTarget()
+          .(MethodDecl)
+          .hasQualifiedName(["UIWebView", "WebView"], "stringByEvaluatingJavaScript(from:)")
+    ).getArgument(0).getExpr() = this.asExpr()
+  }
+}
+
+class JSContext extends Sink {
+  JSContext() {
+    any(CallExpr ce |
+      ce.getStaticTarget()
+          .(MethodDecl)
+          .hasQualifiedName("JSContext", ["evaluateScript(_:)", "evaluateScript(_:withSourceURL:)"])
+    ).getArgument(0).getExpr() = this.asExpr()
+  }
+}
+
+class JSEvaluateScript extends Sink {
+  JSEvaluateScript() {
+    any(CallExpr ce |
+      ce.getStaticTarget().(FreeFunctionDecl).hasName("JSEvaluateScript(_:_:_:_:_:_:)")
+    ).getArgument(1).getExpr() = this.asExpr()
+  }
+}
+
+/**
+ * A taint configuration from taint sources to sinks for this query.
+ */
+class UnsafeJsEvalConfig extends TaintTracking::Configuration {
+  UnsafeJsEvalConfig() { this = "UnsafeJsEvalConfig" }
+
+  override predicate isSource(DataFlow::Node node) { node instanceof Source }
+
+  override predicate isSink(DataFlow::Node node) { node instanceof Sink }
+
+  // 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("WKUserScript",
+                [
+                  "init(source:injectionTime:forMainFrameOnly:)",
+                  "init(source:injectionTime:forMainFrameOnly:in:)"
+                ])
+        ).getArgument(0)
+      or
+      arg =
+        any(CallExpr ce | ce.getStaticTarget().(MethodDecl).hasQualifiedName("Data", "init(_:)"))
+            .getArgument(0)
+      or
+      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() = "String" and
+      member.getName() = ["utf8", "utf16", "utf8CString"]
+      or
+      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
+    )
+  }
+}
+
+from
+  UnsafeJsEvalConfig config, DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode, Sink sink
+where
+  config.hasFlowPath(sourceNode, sinkNode) and
+  sink = sinkNode.getNode()
+select sink, sourceNode, sinkNode, "Evaluation of uncontrolled JavaScript from a remote source."

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

@@ -0,0 +1,6 @@
+let webview: WKWebView
+let remoteData = try String(contentsOf: URL(string: "http://example.com/evil.json")!)
+
+...
+
+_ = try await webview.evaluateJavaScript("console.log(" + remoteData + ")") // BAD

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

@@ -0,0 +1,10 @@
+let webview: WKWebView
+let remoteData = try String(contentsOf: URL(string: "http://example.com/evil.json")!)
+
+...
+
+_ = try await webview.callAsyncJavaScript(
+    "console.log(data)",
+    arguments: ["data": remoteData], // GOOD
+    contentWorld: .page
+)