From 1e0c6811a4f072242d1160d528b41b15403712f5 Mon Sep 17 00:00:00 2001
From: Ed Minnix <egregius313@github.com>
Date: Tue, 21 Mar 2023 16:36:26 -0400
Subject: [PATCH] Refactor UnsafeAndroidAccess

---
 .../java/security/UnsafeAndroidAccessQuery.qll     | 14 +++++++++++++-
 .../Security/CWE/CWE-749/UnsafeAndroidAccess.ql    |  6 +++---
 .../security/CWE-749/UnsafeAndroidAccessTest.ql    |  2 +-
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/java/ql/lib/semmle/code/java/security/UnsafeAndroidAccessQuery.qll b/java/ql/lib/semmle/code/java/security/UnsafeAndroidAccessQuery.qll
index 6bd9833047d..3d4a5bd8057 100644
--- a/java/ql/lib/semmle/code/java/security/UnsafeAndroidAccessQuery.qll
+++ b/java/ql/lib/semmle/code/java/security/UnsafeAndroidAccessQuery.qll
@@ -7,9 +7,11 @@ import semmle.code.java.security.RequestForgery
 import semmle.code.java.security.UnsafeAndroidAccess
 
 /**
+ * DEPRECATED: Use `FetchUntrustedResourceFlow` instead.
+ *
  * A taint configuration tracking flow from untrusted inputs to a resource fetching call.
  */
-class FetchUntrustedResourceConfiguration extends TaintTracking::Configuration {
+deprecated class FetchUntrustedResourceConfiguration extends TaintTracking::Configuration {
   FetchUntrustedResourceConfiguration() { this = "FetchUntrustedResourceConfiguration" }
 
   override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -20,3 +22,13 @@ class FetchUntrustedResourceConfiguration extends TaintTracking::Configuration {
     sanitizer instanceof RequestForgerySanitizer
   }
 }
+
+private module FetchUntrustedResourceConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof UrlResourceSink }
+
+  predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof RequestForgerySanitizer }
+}
+
+module FetchUntrustedResourceFlow = TaintTracking::Make<FetchUntrustedResourceConfig>;
diff --git a/java/ql/src/Security/CWE/CWE-749/UnsafeAndroidAccess.ql b/java/ql/src/Security/CWE/CWE-749/UnsafeAndroidAccess.ql
index 277b1b5790e..c397bf74b93 100644
--- a/java/ql/src/Security/CWE/CWE-749/UnsafeAndroidAccess.ql
+++ b/java/ql/src/Security/CWE/CWE-749/UnsafeAndroidAccess.ql
@@ -14,9 +14,9 @@
 
 import java
 import semmle.code.java.security.UnsafeAndroidAccessQuery
-import DataFlow::PathGraph
+import FetchUntrustedResourceFlow::PathGraph
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, FetchUntrustedResourceConfiguration conf
-where conf.hasFlowPath(source, sink)
+from FetchUntrustedResourceFlow::PathNode source, FetchUntrustedResourceFlow::PathNode sink
+where FetchUntrustedResourceFlow::hasFlowPath(source, sink)
 select sink.getNode(), source, sink, "Unsafe resource fetching in Android WebView due to $@.",
   source.getNode(), sink.getNode().(UrlResourceSink).getSinkType()
diff --git a/java/ql/test/query-tests/security/CWE-749/UnsafeAndroidAccessTest.ql b/java/ql/test/query-tests/security/CWE-749/UnsafeAndroidAccessTest.ql
index 7305d6aeb6f..158c00e5566 100644
--- a/java/ql/test/query-tests/security/CWE-749/UnsafeAndroidAccessTest.ql
+++ b/java/ql/test/query-tests/security/CWE-749/UnsafeAndroidAccessTest.ql
@@ -9,7 +9,7 @@ class UnsafeAndroidAccessTest extends InlineExpectationsTest {
 
   override predicate hasActualResult(Location location, string element, string tag, string value) {
     tag = "hasUnsafeAndroidAccess" and
-    exists(DataFlow::Node sink, FetchUntrustedResourceConfiguration conf | conf.hasFlowTo(sink) |
+    exists(DataFlow::Node sink | FetchUntrustedResourceFlow::hasFlowTo(sink) |
       sink.getLocation() = location and
       element = sink.toString() and
       value = ""