From 768102ee925be402d4dfcabeeaa8d69b54f4848e Mon Sep 17 00:00:00 2001
From: Ed Minnix <egregius313@github.com>
Date: Thu, 16 Mar 2023 14:38:54 -0400
Subject: [PATCH] Refactor java/android/webview-debugging-enabled

---
 ...y.qll => WebviewDebuggingEnabledQuery.qll} | 30 +++++++++++++++++--
 .../CWE/CWE-489/WebviewDebuggingEnabled.ql    |  8 ++---
 .../WebviewDebuggingEnabled.ql                | 21 +++++++------
 3 files changed, 44 insertions(+), 15 deletions(-)
 rename java/ql/lib/semmle/code/java/security/{WebviewDubuggingEnabledQuery.qll => WebviewDebuggingEnabledQuery.qll} (59%)

diff --git a/java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll b/java/ql/lib/semmle/code/java/security/WebviewDebuggingEnabledQuery.qll
similarity index 59%
rename from java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll
rename to java/ql/lib/semmle/code/java/security/WebviewDebuggingEnabledQuery.qll
index 92071c2af1a..858da08ea01 100644
--- a/java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll
+++ b/java/ql/lib/semmle/code/java/security/WebviewDebuggingEnabledQuery.qll
@@ -18,8 +18,12 @@ private predicate isDebugCheck(Expr ex) {
   )
 }
 
-/** A configuration to find instances of `setWebContentDebuggingEnabled` called with `true` values. */
-class WebviewDebugEnabledConfig extends DataFlow::Configuration {
+/**
+ * DEPRECATED: Use `WebviewDebugEnabledFlow` instead.
+ *
+ * A configuration to find instances of `setWebContentDebuggingEnabled` called with `true` values.
+ */
+deprecated class WebviewDebugEnabledConfig extends DataFlow::Configuration {
   WebviewDebugEnabledConfig() { this = "WebviewDebugEnabledConfig" }
 
   override predicate isSource(DataFlow::Node node) {
@@ -39,3 +43,25 @@ class WebviewDebugEnabledConfig extends DataFlow::Configuration {
     node.getEnclosingCallable().getDeclaringType() instanceof NonSecurityTestClass
   }
 }
+
+/** A configuration to find instances of `setWebContentDebuggingEnabled` called with `true` values. */
+private module WebviewDebugEnabledConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) {
+    node.asExpr().(BooleanLiteral).getBooleanValue() = true
+  }
+
+  predicate isSink(DataFlow::Node node) {
+    exists(MethodAccess ma |
+      ma.getMethod().hasQualifiedName("android.webkit", "WebView", "setWebContentsDebuggingEnabled") and
+      node.asExpr() = ma.getArgument(0)
+    )
+  }
+
+  predicate isBarrier(DataFlow::Node node) {
+    exists(Guard debug | isDebugCheck(debug) and debug.controls(node.asExpr().getBasicBlock(), _))
+    or
+    node.getEnclosingCallable().getDeclaringType() instanceof NonSecurityTestClass
+  }
+}
+
+module WebviewDebugEnabledFlow = DataFlow::Make<WebviewDebugEnabledConfig>;
diff --git a/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql
index a8f7f81d832..cafa616e2d1 100644
--- a/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql
+++ b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql
@@ -11,9 +11,9 @@
  */
 
 import java
-import semmle.code.java.security.WebviewDubuggingEnabledQuery
-import DataFlow::PathGraph
+import semmle.code.java.security.WebviewDebuggingEnabledQuery
+import WebviewDebugEnabledFlow::PathGraph
 
-from WebviewDebugEnabledConfig conf, DataFlow::PathNode source, DataFlow::PathNode sink
-where conf.hasFlowPath(source, sink)
+from WebviewDebugEnabledFlow::PathNode source, WebviewDebugEnabledFlow::PathNode sink
+where WebviewDebugEnabledFlow::hasFlowPath(source, sink)
 select sink, source, sink, "Webview debugging is enabled."
diff --git a/java/ql/test/query-tests/security/CWE-489/webview-debugging/WebviewDebuggingEnabled.ql b/java/ql/test/query-tests/security/CWE-489/webview-debugging/WebviewDebuggingEnabled.ql
index aeceb941e09..35d237bf134 100644
--- a/java/ql/test/query-tests/security/CWE-489/webview-debugging/WebviewDebuggingEnabled.ql
+++ b/java/ql/test/query-tests/security/CWE-489/webview-debugging/WebviewDebuggingEnabled.ql
@@ -1,15 +1,18 @@
 import java
-import TestUtilities.InlineFlowTest
-import semmle.code.java.security.WebviewDubuggingEnabledQuery
+import TestUtilities.InlineExpectationsTest
+import semmle.code.java.security.WebviewDebuggingEnabledQuery
 
-class EnableLegacy extends EnableLegacyConfiguration {
-  EnableLegacy() { exists(this) }
-}
+class HasFlowTest extends InlineExpectationsTest {
+  HasFlowTest() { this = "HasFlowTest" }
 
-class HasFlowTest extends InlineFlowTest {
-  override DataFlow::Configuration getTaintFlowConfig() { none() }
+  override string getARelevantTag() { result = "hasValueFlow" }
 
-  override DataFlow::Configuration getValueFlowConfig() {
-    result = any(WebviewDebugEnabledConfig c)
+  override predicate hasActualResult(Location location, string element, string tag, string value) {
+    tag = "hasValueFlow" and
+    exists(DataFlow::Node sink | WebviewDebugEnabledFlow::hasFlowTo(sink) |
+      location = sink.getLocation() and
+      element = "sink" and
+      value = ""
+    )
   }
 }