Merge pull request github#12044 from atorralba/atorralba/webview-models

Swift: Add new source and flow step related to WkWebView

Author: atorralba

swift/ql/lib/codeql/swift/frameworks/StandardLibrary/WebView.qll

@@ -33,6 +33,58 @@ private class WKScriptMessageBodyInheritsTaint extends TaintInheritingContent,
   }
 }
 
+/**
+ * A type or extension delcaration that adopts the protocol `WKNavigationDelegate`.
+ */
+private class AdoptsWkNavigationDelegate extends Decl {
+  AdoptsWkNavigationDelegate() {
+    exists(ProtocolDecl delegate |
+      this.(ExtensionDecl).getAProtocol().getABaseTypeDecl*() = delegate or
+      this.(NominalTypeDecl).getABaseTypeDecl*() = delegate
+    |
+      delegate.getName() = "WKNavigationDelegate"
+    )
+  }
+}
+
+/**
+ * A source for the `decidePolicyFor` parameter of the `webView` method of a type adopting `WKNavigationDelegate`.
+ */
+private class WKNavigationDelegateSource extends RemoteFlowSource {
+  WKNavigationDelegateSource() {
+    exists(ParamDecl p, FuncDecl f, AdoptsWkNavigationDelegate t |
+      t.getAMember() = f and
+      f.getName() =
+        [
+          "webView(_:decidePolicyFor:preferences:decisionHandler:)",
+          "webView(_:decidePolicyFor:decisionHandler:)"
+        ] and
+      p.getDeclaringFunction() = f and
+      p.getIndex() = 1 and
+      this.(DataFlow::ParameterNode).getParameter() = p
+    )
+  }
+
+  override string getSourceType() { result = "Navigation action of a WebView" }
+}
+
+/**
+ * A taint step implying that, if a `WKNavigationAction` is tainted, its `request` field is also tainted.
+ */
+private class WKNavigationActionTaintStep extends AdditionalTaintStep {
+  override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
+    exists(MemberRefExpr e, Expr self, VarDecl member |
+      self.getType().getName() = "WKNavigationAction" and
+      member.getName() = "request"
+    |
+      e.getBase() = self and
+      e.getMember() = member and
+      n1.asExpr() = self and
+      n2.asExpr() = e
+    )
+  }
+}
+
 /**
  * A model for `JSContext` sources. `JSContext` acts as a bridge between JavaScript and
  * native code, so any object obtained from it has the potential of being tainted by a malicious

swift/ql/test/library-tests/dataflow/flowsources/FlowSources.expected

@@ -94,10 +94,16 @@
 | url.swift:53:15:53:19 | .resourceBytes | external |
 | url.swift:60:15:60:19 | .lines | external |
 | url.swift:67:16:67:22 | .lines | external |
-| webview.swift:20:82:20:102 | message | external |
-| webview.swift:25:5:25:13 | .globalObject | external |
-| webview.swift:26:5:26:39 | call to objectForKeyedSubscript(_:) | external |
-| webview.swift:39:9:39:9 | .tainted | Member of a type exposed through JSExport |
-| webview.swift:43:10:43:10 | self | Member of a type exposed through JSExport |
-| webview.swift:43:18:43:24 | arg1 | Member of a type exposed through JSExport |
-| webview.swift:43:29:43:35 | arg2 | Member of a type exposed through JSExport |
+| webview.swift:13:32:13:49 | decidePolicyFor | Navigation action of a WebView |
+| webview.swift:14:32:14:49 | decidePolicyFor | Navigation action of a WebView |
+| webview.swift:41:82:41:102 | message | external |
+| webview.swift:46:5:46:13 | .globalObject | external |
+| webview.swift:47:5:47:39 | call to objectForKeyedSubscript(_:) | external |
+| webview.swift:60:9:60:9 | .tainted | Member of a type exposed through JSExport |
+| webview.swift:64:10:64:10 | self | Member of a type exposed through JSExport |
+| webview.swift:64:18:64:24 | arg1 | Member of a type exposed through JSExport |
+| webview.swift:64:29:64:35 | arg2 | Member of a type exposed through JSExport |
+| webview.swift:72:32:72:49 | decidePolicyFor | Navigation action of a WebView |
+| webview.swift:73:32:73:49 | decidePolicyFor | Navigation action of a WebView |
+| webview.swift:79:32:79:49 | decidePolicyFor | Navigation action of a WebView |
+| webview.swift:80:32:80:49 | decidePolicyFor | Navigation action of a WebView |

swift/ql/test/library-tests/dataflow/flowsources/webview.swift

@@ -1,18 +1,39 @@
 
 // --- stubs ---
+
 class WKUserContentController {}
+
 class WKScriptMessage {}
+
 protocol WKScriptMessageHandler {
     func userContentController(_ userContentController: WKUserContentController, didReceive message: WKScriptMessage)
 }
+
+protocol WKNavigationDelegate {
+    func webView(_: WKWebView, decidePolicyFor: WKNavigationAction, preferences: WKWebpagePreferences, decisionHandler: (WKNavigationActionPolicy, WKWebpagePreferences) -> Void)
+    func webView(_: WKWebView, decidePolicyFor: WKNavigationAction, decisionHandler: (WKNavigationActionPolicy) -> Void)
+}
+
+class WKWebView {}
+
+class WKNavigationAction {}
+
+class WKWebpagePreferences {}
+
+enum WKNavigationActionPolicy {}
+
 protocol NSCopying {}
+
 protocol NSObjectProtocol {}
+
 class JSValue {}
+
 class JSContext {
     var globalObject: JSValue { get { return JSValue() } }
     func objectForKeyedSubscript(_: Any!) -> JSValue! { return JSValue() } 
     func setObject(_: Any, forKeyedSubscript: (NSCopying & NSObjectProtocol) ) {}
 }
+
 protocol JSExport {}
 
 // --- tests ---
@@ -46,3 +67,15 @@ class ExportedImpl : Exported {
     func notTainted(arg1: Any, arg2: Any) {
     }
 } 
+
+class WebViewDelegate : WKNavigationDelegate {
+    func webView(_: WKWebView, decidePolicyFor: WKNavigationAction, preferences: WKWebpagePreferences, decisionHandler: (WKNavigationActionPolicy, WKWebpagePreferences) -> Void) {} // SOURCE
+    func webView(_: WKWebView, decidePolicyFor: WKNavigationAction, decisionHandler: (WKNavigationActionPolicy) -> Void) {} // SOURCE
+}
+
+class Extended {}
+
+extension Extended : WKNavigationDelegate {
+    func webView(_: WKWebView, decidePolicyFor: WKNavigationAction, preferences: WKWebpagePreferences, decisionHandler: (WKNavigationActionPolicy, WKWebpagePreferences) -> Void) {} // SOURCE
+    func webView(_: WKWebView, decidePolicyFor: WKNavigationAction, decisionHandler: (WKNavigationActionPolicy) -> Void) {} // SOURCE
+}