Merge pull request #13705 from atorralba/atorralba/java/android-unsafe-fetch-apply

Java: Add support for Kotlin's `apply` to java/android/unsafe-android-wevbiew-fetch

Author: atorralba

java/ql/lib/ext/android.webkit.model.yml

@@ -3,13 +3,13 @@ extensions:
       pack: codeql/java-all
       extensible: sourceModel
     data:
-      - ["android.webkit", "WebView", False, "getOriginalUrl", "()", "", "ReturnValue", "remote", "manual"]
-      - ["android.webkit", "WebView", False, "getUrl", "()", "", "ReturnValue", "remote", "manual"]
+      - ["android.webkit", "WebView", True, "getOriginalUrl", "()", "", "ReturnValue", "remote", "manual"]
+      - ["android.webkit", "WebView", True, "getUrl", "()", "", "ReturnValue", "remote", "manual"]
   - addsTo:
       pack: codeql/java-all
       extensible: sinkModel
     data:
       # Models representing methods susceptible to XSS attacks.
-      - ["android.webkit", "WebView", False, "evaluateJavascript", "", "", "Argument[0]", "js-injection", "manual"]
-      - ["android.webkit", "WebView", False, "loadData", "", "", "Argument[0]", "html-injection", "manual"]
-      - ["android.webkit", "WebView", False, "loadDataWithBaseURL", "", "", "Argument[1]", "html-injection", "manual"]
+      - ["android.webkit", "WebView", True, "evaluateJavascript", "", "", "Argument[0]", "js-injection", "manual"]
+      - ["android.webkit", "WebView", True, "loadData", "", "", "Argument[0]", "html-injection", "manual"]
+      - ["android.webkit", "WebView", True, "loadDataWithBaseURL", "", "", "Argument[1]", "html-injection", "manual"]

java/ql/lib/semmle/code/java/security/UnsafeAndroidAccess.qll

@@ -5,6 +5,7 @@
 import java
 private import semmle.code.java.dataflow.DataFlow
 private import semmle.code.java.frameworks.android.WebView
+private import semmle.code.java.frameworks.kotlin.Kotlin
 
 /**
  * A sink that represents a method that fetches a web resource in Android.
@@ -62,10 +63,26 @@ private class WebViewRef extends Element {
       t.isOwnInstanceAccess() or t.getInstanceAccess().isEnclosingInstanceAccess(this)
     )
     or
-    result = DataFlow::exprNode(this.(Variable).getAnAccess())
+    exists(Variable v | result.asExpr() = v.getAnAccess() |
+      v = this
+      or
+      applyReceiverVariable(this, v)
+    )
   }
 }
 
+/**
+ * Holds if `p` is the lambda parameter that holds the receiver of an `apply` expression in Kotlin,
+ * and `v` is the variable of the receiver in the outer scope.
+ */
+private predicate applyReceiverVariable(Parameter p, Variable v) {
+  exists(LambdaExpr lambda, KotlinApply apply |
+    p.getCallable() = lambda.asMethod() and
+    lambda = apply.getLambdaArg() and
+    v = apply.getReceiver().(VarAccess).getVariable()
+  )
+}
+
 /**
  * Holds if a `WebViewLoadUrlMethod` is called on an access of `webview`
  * with `urlArg` as its first argument.

java/ql/src/change-notes/2023-07-10-unsafe-android-access-apply.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The query "Unsafe resource fetching in Android WebView" (`java/android/unsafe-android-webview-fetch`) now recognizes WebViews where `setJavascriptEnabled`, `setAllowFileAccess`, `setAllowUniversalAccessFromFileURLs`, and/or `setAllowFileAccessFromFileURLs` are set inside the function block of the Kotlin `apply` function.

java/ql/test/query-tests/security/CWE-749/UnsafeActivityKt.kt

@@ -9,12 +9,19 @@ import android.webkit.WebViewClient
 class UnsafeActivityKt : Activity() {
     override fun onCreate(savedInstanceState : Bundle) {
 
+		val src : String = intent.extras.getString("url")
+
 		val wv = findViewById<WebView>(-1)
 		// Implicit not-nulls happening here
 		wv.settings.setJavaScriptEnabled(true)
 		wv.settings.setAllowFileAccessFromFileURLs(true)
 
-		val thisUrl : String = intent.extras.getString("url")
-		wv.loadUrl(thisUrl) // $ hasUnsafeAndroidAccess
+		wv.loadUrl(src) // $ hasUnsafeAndroidAccess
+
+		val wv2 = findViewById<WebView>(-1)
+		wv2.apply {
+			settings.setJavaScriptEnabled(true)
+		}
+		wv2.loadUrl(src) // $ hasUnsafeAndroidAccess
     }
 }