From 16529cdd18908f6c7e3e1251faac16d0d1dd1d1c Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Mon, 10 Jul 2023 17:17:48 +0200 Subject: [PATCH 1/2] Add failing test --- .../query-tests/security/CWE-749/UnsafeActivityKt.kt | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/java/ql/test/query-tests/security/CWE-749/UnsafeActivityKt.kt b/java/ql/test/query-tests/security/CWE-749/UnsafeActivityKt.kt index d20845f5c77..32004071c3c 100644 --- a/java/ql/test/query-tests/security/CWE-749/UnsafeActivityKt.kt +++ b/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(-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(-1) + wv2.apply { + settings.setJavaScriptEnabled(true) + } + wv2.loadUrl(src) // $ hasUnsafeAndroidAccess } } From ce600367dfc7a68bb0f9f8e834130ac21cc01296 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Mon, 10 Jul 2023 17:21:56 +0200 Subject: [PATCH 2/2] Java: Add support for Kotlin's apply to java/android/unsafe-android-webview-fetch --- java/ql/lib/ext/android.webkit.model.yml | 10 +++++----- .../java/security/UnsafeAndroidAccess.qll | 19 ++++++++++++++++++- .../2023-07-10-unsafe-android-access-apply.md | 4 ++++ 3 files changed, 27 insertions(+), 6 deletions(-) create mode 100644 java/ql/src/change-notes/2023-07-10-unsafe-android-access-apply.md diff --git a/java/ql/lib/ext/android.webkit.model.yml b/java/ql/lib/ext/android.webkit.model.yml index d88199c04cb..bb375507194 100644 --- a/java/ql/lib/ext/android.webkit.model.yml +++ b/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"] diff --git a/java/ql/lib/semmle/code/java/security/UnsafeAndroidAccess.qll b/java/ql/lib/semmle/code/java/security/UnsafeAndroidAccess.qll index ba162ede986..9a85a771406 100644 --- a/java/ql/lib/semmle/code/java/security/UnsafeAndroidAccess.qll +++ b/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. diff --git a/java/ql/src/change-notes/2023-07-10-unsafe-android-access-apply.md b/java/ql/src/change-notes/2023-07-10-unsafe-android-access-apply.md new file mode 100644 index 00000000000..ddb5f7c055c --- /dev/null +++ b/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.