Decouple UnsafeAndroidAccess.qll to reuse the taint tracking configuration

Author: atorralba

java/ql/src/Security/CWE/CWE-749/UnsafeAndroidAccess.ql

@@ -12,26 +12,9 @@
  */
 
 import java
-import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.security.RequestForgeryConfig
-import semmle.code.java.security.UnsafeAndroidAccess
+import semmle.code.java.security.UnsafeAndroidAccessQuery
 import DataFlow::PathGraph
 
-/**
- * Taint configuration tracking flow from untrusted inputs to a resource fetching call.
- */
-class FetchUntrustedResourceConfiguration extends TaintTracking::Configuration {
-  FetchUntrustedResourceConfiguration() { this = "FetchUntrustedResourceConfiguration" }
-
-  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof UrlResourceSink }
-
-  override predicate isSanitizer(DataFlow::Node sanitizer) {
-    sanitizer instanceof RequestForgerySanitizer
-  }
-}
-
 from DataFlow::PathNode source, DataFlow::PathNode sink, FetchUntrustedResourceConfiguration conf
 where conf.hasFlowPath(source, sink)
 select sink.getNode(), source, sink, "Unsafe resource fetching in Android webview due to $@.",

java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll

@@ -98,7 +98,6 @@ private module Frameworks {
   private import semmle.code.java.security.InformationLeak
   private import semmle.code.java.security.JexlInjectionSinkModels
   private import semmle.code.java.security.LdapInjection
-  private import semmle.code.java.security.UnsafeAndroidAccess
   private import semmle.code.java.security.XPath
   private import semmle.code.java.frameworks.android.SQLite
   private import semmle.code.java.frameworks.Jdbc

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

@@ -3,9 +3,9 @@
  */
 
 import java
-import semmle.code.java.frameworks.android.WebView
-import semmle.code.java.dataflow.DataFlow
-import semmle.code.java.dataflow.ExternalFlow
+private import semmle.code.java.frameworks.android.WebView
+private import semmle.code.java.dataflow.DataFlow
+private import semmle.code.java.dataflow.ExternalFlow
 
 /**
  * A sink that represents a method that fetches a web resource in Android.
@@ -19,17 +19,6 @@ abstract class UrlResourceSink extends DataFlow::Node {
   abstract string getSinkType();
 }
 
-/** CSV sink models representing methods susceptible to Unsafe Resource Fetching attacks. */
-private class DefaultUrlResourceSinkModel extends SinkModelCsv {
-  override predicate row(string row) {
-    row =
-      [
-        "android.webkit;WebView;true;loadUrl;;;Argument[0];unsafe-android-access",
-        "android.webkit;WebView;true;postUrl;;;Argument[0];unsafe-android-access"
-      ]
-  }
-}
-
 /**
  * Cross-origin access enabled resource fetch.
  *
@@ -57,9 +46,10 @@ private class CrossOriginUrlResourceSink extends JavaScriptEnabledUrlResourceSin
  */
 private class JavaScriptEnabledUrlResourceSink extends UrlResourceSink {
   JavaScriptEnabledUrlResourceSink() {
-    sinkNode(this, "unsafe-android-access") and
-    exists(VarAccess webviewVa, MethodAccess getSettingsMa, Variable v |
-      this.asExpr().(Argument).getCall().getQualifier() = webviewVa and
+    exists(MethodAccess loadUrl, VarAccess webviewVa, MethodAccess getSettingsMa, Variable v |
+      loadUrl.getArgument(0) = this.asExpr() and
+      loadUrl.getMethod() instanceof WebViewLoadUrlMethod and
+      loadUrl.getQualifier() = webviewVa and
       getSettingsMa.getMethod() instanceof WebViewGetSettingsMethod and
       webviewVa.getVariable().getAnAccess() = getSettingsMa.getQualifier() and
       v.getAnAssignedValue() = getSettingsMa and

java/ql/src/semmle/code/java/security/UnsafeAndroidAccessQuery.qll

@@ -0,0 +1,22 @@
+/** Provides taint tracking configurations to be used in Unsafe Resource Fetching queries. */
+
+import java
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.dataflow.TaintTracking
+import semmle.code.java.security.RequestForgery
+import semmle.code.java.security.UnsafeAndroidAccess
+
+/**
+ * Taint configuration tracking flow from untrusted inputs to a resource fetching call.
+ */
+class FetchUntrustedResourceConfiguration extends TaintTracking::Configuration {
+  FetchUntrustedResourceConfiguration() { this = "FetchUntrustedResourceConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof UrlResourceSink }
+
+  override predicate isSanitizer(DataFlow::Node sanitizer) {
+    sanitizer instanceof RequestForgerySanitizer
+  }
+}

java/ql/test/query-tests/security/CWE-749/UnsafeAndroidAccessTest.ql

@@ -1,30 +1,17 @@
 import java
-import semmle.code.java.dataflow.DataFlow
-import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.security.RequestForgeryConfig
-import semmle.code.java.security.UnsafeAndroidAccess
+import semmle.code.java.security.UnsafeAndroidAccessQuery
 import TestUtilities.InlineExpectationsTest
 
-class Conf extends TaintTracking::Configuration {
-  Conf() { this = "qltest:cwe:unsafe-android-access" }
-
-  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof UrlResourceSink }
-
-  override predicate isSanitizer(DataFlow::Node sanitizer) {
-    sanitizer instanceof RequestForgerySanitizer
-  }
-}
-
 class UnsafeAndroidAccessTest extends InlineExpectationsTest {
   UnsafeAndroidAccessTest() { this = "HasUnsafeAndroidAccess" }
 
   override string getARelevantTag() { result = "hasUnsafeAndroidAccess" }
 
   override predicate hasActualResult(Location location, string element, string tag, string value) {
     tag = "hasUnsafeAndroidAccess" and
-    exists(DataFlow::Node src, DataFlow::Node sink, Conf conf | conf.hasFlow(src, sink) |
+    exists(DataFlow::Node src, DataFlow::Node sink, FetchUntrustedResourceConfiguration conf |
+      conf.hasFlow(src, sink)
+    |
       sink.getLocation() = location and
       element = sink.toString() and
       value = ""