From 20b2956322ee0a57900097ec5b2feda21db7ed63 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Fri, 26 Aug 2022 16:34:06 +0100
Subject: [PATCH 01/13] Add webview debugging query

---
 .../security/WebviewDubuggingEnabledQuery.qll | 43 +++++++++++++++++++
 .../CWE/CWE-489/WebviewDebuggingEnabled.ql    | 20 +++++++++
 2 files changed, 63 insertions(+)
 create mode 100644 java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll
 create mode 100644 java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql

diff --git a/java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll b/java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll
new file mode 100644
index 00000000000..ca1412886d7
--- /dev/null
+++ b/java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll
@@ -0,0 +1,43 @@
+/** Definitions for the Android Webview Debugging Enabled query */
+
+import java
+import semmle.code.java.dataflow.DataFlow
+import semmle.code.java.controlflow.Guards
+import semmle.code.java.security.SecurityTests
+
+/** Holds if `ex` looks like a check that this is a debug build. */
+private predicate isDebugCheck(Expr ex) {
+  exists(Expr subex, string debug |
+    debug.toLowerCase().matches("%debug%") and
+    subex.getParent*() = ex
+  |
+    subex.(VarAccess).getVariable().getName() = debug
+    or
+    subex.(MethodAccess).getMethod().hasName("getProperty") and
+    subex.(MethodAccess).getAnArgument().(CompileTimeConstantExpr).getStringValue() = debug
+  )
+}
+
+/** Configuration to find instances of `setWebContentDebuggingEnabled` called with `true` values. */
+class WebviewDebugEnabledConfig extends DataFlow::Configuration {
+  WebviewDebugEnabledConfig() { this = "WebviewDebugEnabledConfig" }
+
+  override predicate isSource(DataFlow::Node node) {
+    node.asExpr().(BooleanLiteral).getBooleanValue() = true
+  }
+
+  override predicate isSink(DataFlow::Node node) {
+    exists(MethodAccess ma |
+      ma.getMethod().hasQualifiedName("android.webkit", "WebView", "setWebContentsDebuggingEnabled") and
+      node.asExpr() = ma.getArgument(0)
+    )
+  }
+
+  override predicate isBarrier(DataFlow::Node node) {
+    not node.getType() instanceof BooleanType
+    or
+    exists(Guard debug | isDebugCheck(debug) and debug.controls(node.asExpr().getBasicBlock(), _))
+    or
+    node.getEnclosingCallable().getDeclaringType() instanceof NonSecurityTestClass
+  }
+}
diff --git a/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql
new file mode 100644
index 00000000000..8355ce76412
--- /dev/null
+++ b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql
@@ -0,0 +1,20 @@
+/**
+ * @name Android Webview debugging enabled
+ * @description Webview debugging should not be enabled in production builds.
+ * @kind path-problem
+ * @problem.severity warning
+ * @security-severity 7.2
+ * @id java/android/webview-debugging-enabled
+ * @tags security
+ *       external/cwe/cwe-489
+ * @precision high
+ */
+
+import java
+import semmle.code.java.security.WebviewDubuggingEnabledQuery
+import semmle.code.java.dataflow.DataFlow
+import DataFlow::PathGraph
+
+from WebviewDebugEnabledConfig conf, DataFlow::PathNode source, DataFlow::PathNode sink
+where conf.hasFlowPath(source, sink)
+select sink, source, sink, "Webview debugging is enabled here."

From d32540469b5381f7f54ba4c141f5340efabfaf32 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Fri, 26 Aug 2022 16:39:56 +0100
Subject: [PATCH 02/13] Moved existing tests to subdirectory

---
 .../CWE-489/{ => debuggable-attribute}/AndroidManifest.xml      | 0
 .../DebuggableAttributeEnabledTest.expected                     | 0
 .../DebuggableAttributeEnabledTest.ql                           | 0
 .../security/CWE-489/{ => debuggable-attribute}/Test.java       | 0
 .../{ => debuggable-attribute}/TestFalse/AndroidManifest.xml    | 0
 .../{ => debuggable-attribute}/TestNotSet/AndroidManifest.xml   | 0
 .../{ => debuggable-attribute}/Testbuild/AndroidManifest.xml    | 0
 .../security/CWE-489/{ => debuggable-attribute}/options         | 2 +-
 8 files changed, 1 insertion(+), 1 deletion(-)
 rename java/ql/test/query-tests/security/CWE-489/{ => debuggable-attribute}/AndroidManifest.xml (100%)
 rename java/ql/test/query-tests/security/CWE-489/{ => debuggable-attribute}/DebuggableAttributeEnabledTest.expected (100%)
 rename java/ql/test/query-tests/security/CWE-489/{ => debuggable-attribute}/DebuggableAttributeEnabledTest.ql (100%)
 rename java/ql/test/query-tests/security/CWE-489/{ => debuggable-attribute}/Test.java (100%)
 rename java/ql/test/query-tests/security/CWE-489/{ => debuggable-attribute}/TestFalse/AndroidManifest.xml (100%)
 rename java/ql/test/query-tests/security/CWE-489/{ => debuggable-attribute}/TestNotSet/AndroidManifest.xml (100%)
 rename java/ql/test/query-tests/security/CWE-489/{ => debuggable-attribute}/Testbuild/AndroidManifest.xml (100%)
 rename java/ql/test/query-tests/security/CWE-489/{ => debuggable-attribute}/options (67%)

diff --git a/java/ql/test/query-tests/security/CWE-489/AndroidManifest.xml b/java/ql/test/query-tests/security/CWE-489/debuggable-attribute/AndroidManifest.xml
similarity index 100%
rename from java/ql/test/query-tests/security/CWE-489/AndroidManifest.xml
rename to java/ql/test/query-tests/security/CWE-489/debuggable-attribute/AndroidManifest.xml
diff --git a/java/ql/test/query-tests/security/CWE-489/DebuggableAttributeEnabledTest.expected b/java/ql/test/query-tests/security/CWE-489/debuggable-attribute/DebuggableAttributeEnabledTest.expected
similarity index 100%
rename from java/ql/test/query-tests/security/CWE-489/DebuggableAttributeEnabledTest.expected
rename to java/ql/test/query-tests/security/CWE-489/debuggable-attribute/DebuggableAttributeEnabledTest.expected
diff --git a/java/ql/test/query-tests/security/CWE-489/DebuggableAttributeEnabledTest.ql b/java/ql/test/query-tests/security/CWE-489/debuggable-attribute/DebuggableAttributeEnabledTest.ql
similarity index 100%
rename from java/ql/test/query-tests/security/CWE-489/DebuggableAttributeEnabledTest.ql
rename to java/ql/test/query-tests/security/CWE-489/debuggable-attribute/DebuggableAttributeEnabledTest.ql
diff --git a/java/ql/test/query-tests/security/CWE-489/Test.java b/java/ql/test/query-tests/security/CWE-489/debuggable-attribute/Test.java
similarity index 100%
rename from java/ql/test/query-tests/security/CWE-489/Test.java
rename to java/ql/test/query-tests/security/CWE-489/debuggable-attribute/Test.java
diff --git a/java/ql/test/query-tests/security/CWE-489/TestFalse/AndroidManifest.xml b/java/ql/test/query-tests/security/CWE-489/debuggable-attribute/TestFalse/AndroidManifest.xml
similarity index 100%
rename from java/ql/test/query-tests/security/CWE-489/TestFalse/AndroidManifest.xml
rename to java/ql/test/query-tests/security/CWE-489/debuggable-attribute/TestFalse/AndroidManifest.xml
diff --git a/java/ql/test/query-tests/security/CWE-489/TestNotSet/AndroidManifest.xml b/java/ql/test/query-tests/security/CWE-489/debuggable-attribute/TestNotSet/AndroidManifest.xml
similarity index 100%
rename from java/ql/test/query-tests/security/CWE-489/TestNotSet/AndroidManifest.xml
rename to java/ql/test/query-tests/security/CWE-489/debuggable-attribute/TestNotSet/AndroidManifest.xml
diff --git a/java/ql/test/query-tests/security/CWE-489/Testbuild/AndroidManifest.xml b/java/ql/test/query-tests/security/CWE-489/debuggable-attribute/Testbuild/AndroidManifest.xml
similarity index 100%
rename from java/ql/test/query-tests/security/CWE-489/Testbuild/AndroidManifest.xml
rename to java/ql/test/query-tests/security/CWE-489/debuggable-attribute/Testbuild/AndroidManifest.xml
diff --git a/java/ql/test/query-tests/security/CWE-489/options b/java/ql/test/query-tests/security/CWE-489/debuggable-attribute/options
similarity index 67%
rename from java/ql/test/query-tests/security/CWE-489/options
rename to java/ql/test/query-tests/security/CWE-489/debuggable-attribute/options
index dacd3cb21df..33cdc1ea940 100644
--- a/java/ql/test/query-tests/security/CWE-489/options
+++ b/java/ql/test/query-tests/security/CWE-489/debuggable-attribute/options
@@ -1 +1 @@
-//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/google-android-9.0.0
+//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/google-android-9.0.0

From b3d9d0875023a624b52fb139f0376f4de66d7165 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Wed, 31 Aug 2022 10:31:38 +0100
Subject: [PATCH 03/13] Add tests

---
 .../CWE-489/webview-debugging/Test.java       | 23 +++++++++++++++++++
 .../WebviewDebuggingEnabled.expected          |  0
 .../WebviewDebuggingEnabled.ql                | 11 +++++++++
 .../CWE-489/webview-debugging/options         |  1 +
 4 files changed, 35 insertions(+)
 create mode 100644 java/ql/test/query-tests/security/CWE-489/webview-debugging/Test.java
 create mode 100644 java/ql/test/query-tests/security/CWE-489/webview-debugging/WebviewDebuggingEnabled.expected
 create mode 100644 java/ql/test/query-tests/security/CWE-489/webview-debugging/WebviewDebuggingEnabled.ql
 create mode 100644 java/ql/test/query-tests/security/CWE-489/webview-debugging/options

diff --git a/java/ql/test/query-tests/security/CWE-489/webview-debugging/Test.java b/java/ql/test/query-tests/security/CWE-489/webview-debugging/Test.java
new file mode 100644
index 00000000000..3fe75e89388
--- /dev/null
+++ b/java/ql/test/query-tests/security/CWE-489/webview-debugging/Test.java
@@ -0,0 +1,23 @@
+import android.webkit.WebView;
+
+class Test {
+    boolean DEBUG_BUILD;
+
+    void test1() {
+        WebView.setWebContentsDebuggingEnabled(true); // $hasValueFlow
+    }
+
+    void test2(){
+        if (DEBUG_BUILD) {
+            WebView.setWebContentsDebuggingEnabled(true); 
+        }
+    }
+
+    void test3(boolean enabled){
+        WebView.setWebContentsDebuggingEnabled(enabled); // $hasValueFlow
+    }
+
+    void test4(){
+        test3(true);
+    }
+}
\ No newline at end of file
diff --git a/java/ql/test/query-tests/security/CWE-489/webview-debugging/WebviewDebuggingEnabled.expected b/java/ql/test/query-tests/security/CWE-489/webview-debugging/WebviewDebuggingEnabled.expected
new file mode 100644
index 00000000000..e69de29bb2d
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
new file mode 100644
index 00000000000..2d390a22d9f
--- /dev/null
+++ b/java/ql/test/query-tests/security/CWE-489/webview-debugging/WebviewDebuggingEnabled.ql
@@ -0,0 +1,11 @@
+import java
+import TestUtilities.InlineFlowTest
+import semmle.code.java.security.WebviewDubuggingEnabledQuery
+
+class HasFlowTest extends InlineFlowTest {
+  override DataFlow::Configuration getTaintFlowConfig() { none() }
+
+  override DataFlow::Configuration getValueFlowConfig() {
+    result = any(WebviewDebugEnabledConfig c)
+  }
+}
diff --git a/java/ql/test/query-tests/security/CWE-489/webview-debugging/options b/java/ql/test/query-tests/security/CWE-489/webview-debugging/options
new file mode 100644
index 00000000000..3042f155024
--- /dev/null
+++ b/java/ql/test/query-tests/security/CWE-489/webview-debugging/options
@@ -0,0 +1 @@
+//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/google-android-9.0.0
\ No newline at end of file

From f934554143017351b0f568c1f63eb5ca45f21e7d Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Wed, 31 Aug 2022 14:37:59 +0100
Subject: [PATCH 04/13] Add docs + add an additional case

---
 .../security/WebviewDubuggingEnabledQuery.qll |  6 ++-
 .../CWE/CWE-489/WebviewDebuggingEnabled.java  |  7 ++++
 .../CWE/CWE-489/WebviewDebuggingEnabled.qhelp | 38 +++++++++++++++++++
 3 files changed, 50 insertions(+), 1 deletion(-)
 create mode 100644 java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.java
 create mode 100644 java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp

diff --git a/java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll b/java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll
index ca1412886d7..563590725e7 100644
--- a/java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll
+++ b/java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll
@@ -8,7 +8,11 @@ import semmle.code.java.security.SecurityTests
 /** Holds if `ex` looks like a check that this is a debug build. */
 private predicate isDebugCheck(Expr ex) {
   exists(Expr subex, string debug |
-    debug.toLowerCase().matches("%debug%") and
+    (
+      debug.toLowerCase().matches("%debug%")
+      or
+      debug.toLowerCase().matches("%test%")
+    ) and
     subex.getParent*() = ex
   |
     subex.(VarAccess).getVariable().getName() = debug
diff --git a/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.java b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.java
new file mode 100644
index 00000000000..b679cf702c4
--- /dev/null
+++ b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.java
@@ -0,0 +1,7 @@
+// BAD - debugging is always enabled 
+WebView.setWebContentsDebuggingEnabled(true);
+
+// GOOD - debugging is only enabled when this is a debug build, as indicated by the debuggable flag being set.
+if (0 != (getApplicationInfo().flags & ApplicationInfo.FLAG_DEBUGGABLE)) {
+    WebView.setWebContentsDebuggingEnabled(true);
+}
\ No newline at end of file
diff --git a/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp
new file mode 100644
index 00000000000..e5290c42efc
--- /dev/null
+++ b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp
@@ -0,0 +1,38 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>The <code>WebView.setWebContentsDebuggingEnabled</code> method enables or disables the contents of any <code>WebView</code> in the application to be debugged.</p>
+
+<p>Enabling debugging featues could allow for additional entry points or leaking sensitive information. 
+As such, debugging should only be anabled during development, and disabled during production builds.
+</overview>
+<recommendation>
+Ensure that debugging features are not enabled during production builds. 
+If <code>WebView.setWebContentsDebuggingEnabled(true)</code> is used, ensure that it is guarded by a flag indicating that this is a debug build.
+
+</recommendation>
+<example>
+
+<p>In the code below, the BAD case shows debugging always being enabled, 
+whereas the GOOD case only enables debugging if the <code>android:debuggable</code> attribute is set to <code>true</code>.</p>
+
+<sample src="WebviewDebuggingEnabled.java" />
+
+</example>
+<references>
+
+<li>
+  Android Developers: 
+  <a href="https://developer.android.com/reference/android/webkit/WebView.html#setWebContentsDebuggingEnabled(boolean)">setWebContentsDebuggingEnabled</a>.
+</li>
+
+<li>
+  Android Developers: 
+  <a href="https://developer.chrome.com/docs/devtools/remote-debugging/webviews/">Remote debugging WebViews</a>.
+</li>
+
+</references>
+</qhelp>

From 414e0b20b3115735e572b88e0b60b9a69fa57806 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Wed, 31 Aug 2022 14:44:07 +0100
Subject: [PATCH 05/13] Add change note

---
 java/ql/src/change-notes/2022-08-31-webview-dubugging.md | 4 ++++
 1 file changed, 4 insertions(+)
 create mode 100644 java/ql/src/change-notes/2022-08-31-webview-dubugging.md

diff --git a/java/ql/src/change-notes/2022-08-31-webview-dubugging.md b/java/ql/src/change-notes/2022-08-31-webview-dubugging.md
new file mode 100644
index 00000000000..8e6295efeb3
--- /dev/null
+++ b/java/ql/src/change-notes/2022-08-31-webview-dubugging.md
@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* Added a new query, `java/android/webview-debugging-enabled`, to detect instances of WebView debugging being enabled in production builds.
\ No newline at end of file

From eed2df0fb319f771b9fa9913237829af73ee8ae5 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Wed, 31 Aug 2022 15:24:29 +0100
Subject: [PATCH 06/13] Fix qhelp & ql-for-ql errors

---
 .../semmle/code/java/security/WebviewDubuggingEnabledQuery.qll  | 2 +-
 java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp  | 2 +-
 java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql     | 1 -
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll b/java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll
index 563590725e7..e9107ca10b4 100644
--- a/java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll
+++ b/java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll
@@ -22,7 +22,7 @@ private predicate isDebugCheck(Expr ex) {
   )
 }
 
-/** Configuration to find instances of `setWebContentDebuggingEnabled` called with `true` values. */
+/** A configuration to find instances of `setWebContentDebuggingEnabled` called with `true` values. */
 class WebviewDebugEnabledConfig extends DataFlow::Configuration {
   WebviewDebugEnabledConfig() { this = "WebviewDebugEnabledConfig" }
 
diff --git a/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp
index e5290c42efc..04676caa378 100644
--- a/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp
+++ b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp
@@ -7,7 +7,7 @@
 <p>The <code>WebView.setWebContentsDebuggingEnabled</code> method enables or disables the contents of any <code>WebView</code> in the application to be debugged.</p>
 
 <p>Enabling debugging featues could allow for additional entry points or leaking sensitive information. 
-As such, debugging should only be anabled during development, and disabled during production builds.
+As such, debugging should only be anabled during development, and disabled during production builds.</p>
 </overview>
 <recommendation>
 Ensure that debugging features are not enabled during production builds. 
diff --git a/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql
index 8355ce76412..2e94852e211 100644
--- a/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql
+++ b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql
@@ -12,7 +12,6 @@
 
 import java
 import semmle.code.java.security.WebviewDubuggingEnabledQuery
-import semmle.code.java.dataflow.DataFlow
 import DataFlow::PathGraph
 
 from WebviewDebugEnabledConfig conf, DataFlow::PathNode source, DataFlow::PathNode sink

From 6014a75e0eea14b6a797bbb0b21e4d4f6416b1c4 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Wed, 31 Aug 2022 16:19:30 +0100
Subject: [PATCH 07/13] Fix qhelp

---
 .../ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp
index 04676caa378..d8643923bca 100644
--- a/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp
+++ b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp
@@ -10,8 +10,8 @@
 As such, debugging should only be anabled during development, and disabled during production builds.</p>
 </overview>
 <recommendation>
-Ensure that debugging features are not enabled during production builds. 
-If <code>WebView.setWebContentsDebuggingEnabled(true)</code> is used, ensure that it is guarded by a flag indicating that this is a debug build.
+<p>Ensure that debugging features are not enabled during production builds. 
+If <code>WebView.setWebContentsDebuggingEnabled(true)</code> is used, ensure that it is guarded by a flag indicating that this is a debug build.</p>
 
 </recommendation>
 <example>

From a6a500ade23218b6c03f03a4d7856e957038d487 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Thu, 1 Sep 2022 13:54:15 +0100
Subject: [PATCH 08/13] Apply suggestions from code review - doc improvements,
 simplification

Co-authored-by: Tony Torralba <atorralba@users.noreply.github.com>
---
 .../java/security/WebviewDubuggingEnabledQuery.qll     |  6 +-----
 .../Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp | 10 +++++-----
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll b/java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll
index e9107ca10b4..3b937766899 100644
--- a/java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll
+++ b/java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll
@@ -8,11 +8,7 @@ import semmle.code.java.security.SecurityTests
 /** Holds if `ex` looks like a check that this is a debug build. */
 private predicate isDebugCheck(Expr ex) {
   exists(Expr subex, string debug |
-    (
-      debug.toLowerCase().matches("%debug%")
-      or
-      debug.toLowerCase().matches("%test%")
-    ) and
+    debug.toLowerCase().matches(["%debug%", "%test%"]) and
     subex.getParent*() = ex
   |
     subex.(VarAccess).getVariable().getName() = debug
diff --git a/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp
index d8643923bca..d236556d78b 100644
--- a/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp
+++ b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp
@@ -6,18 +6,18 @@
 <overview>
 <p>The <code>WebView.setWebContentsDebuggingEnabled</code> method enables or disables the contents of any <code>WebView</code> in the application to be debugged.</p>
 
-<p>Enabling debugging featues could allow for additional entry points or leaking sensitive information. 
-As such, debugging should only be anabled during development, and disabled during production builds.</p>
+<p>Enabling debugging features could allow for additional entry points or leaking sensitive information. 
+As such, debugging should only be enabled during development, and disabled in production builds.</p>
 </overview>
 <recommendation>
-<p>Ensure that debugging features are not enabled during production builds. 
+<p>Ensure that debugging features are not enabled in production builds. 
 If <code>WebView.setWebContentsDebuggingEnabled(true)</code> is used, ensure that it is guarded by a flag indicating that this is a debug build.</p>
 
 </recommendation>
 <example>
 
-<p>In the code below, the BAD case shows debugging always being enabled, 
-whereas the GOOD case only enables debugging if the <code>android:debuggable</code> attribute is set to <code>true</code>.</p>
+<p>In the code below, the BAD case shows WebView debugging always being enabled, 
+whereas the GOOD case only enables it if the <code>android:debuggable</code> attribute is set to <code>true</code>.</p>
 
 <sample src="WebviewDebuggingEnabled.java" />
 

From 44bd0383391bd30dd6905c5b8dbf73542627807e Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Thu, 8 Sep 2022 18:57:18 +0100
Subject: [PATCH 09/13] Apply docs suggestions from code review

Co-authored-by: Ben Ahmady <32935794+subatoi@users.noreply.github.com>
---
 .../src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp   | 5 ++---
 java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql  | 2 +-
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp
index d236556d78b..bda7fd0926d 100644
--- a/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp
+++ b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp
@@ -6,8 +6,7 @@
 <overview>
 <p>The <code>WebView.setWebContentsDebuggingEnabled</code> method enables or disables the contents of any <code>WebView</code> in the application to be debugged.</p>
 
-<p>Enabling debugging features could allow for additional entry points or leaking sensitive information. 
-As such, debugging should only be enabled during development, and disabled in production builds.</p>
+<p>You should only enable debugging features during development. When you create a production build, you should disable it. If you enable debugging features, this can make your code vulnerable by adding entry points, or leaking sensitive information.
 </overview>
 <recommendation>
 <p>Ensure that debugging features are not enabled in production builds. 
@@ -16,7 +15,7 @@ If <code>WebView.setWebContentsDebuggingEnabled(true)</code> is used, ensure tha
 </recommendation>
 <example>
 
-<p>In the code below, the BAD case shows WebView debugging always being enabled, 
+<p>In the first (bad) example, WebView debugging is always enabled.
 whereas the GOOD case only enables it if the <code>android:debuggable</code> attribute is set to <code>true</code>.</p>
 
 <sample src="WebviewDebuggingEnabled.java" />
diff --git a/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql
index 2e94852e211..efcc8666265 100644
--- a/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql
+++ b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql
@@ -1,6 +1,6 @@
 /**
  * @name Android Webview debugging enabled
- * @description Webview debugging should not be enabled in production builds.
+ * @description Enabling Webview debugging in production builds can expose entry points or leak sensitive information.
  * @kind path-problem
  * @problem.severity warning
  * @security-severity 7.2

From ed8ec894979ae1e9c464e2ae86a836e7b28d4f77 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Tue, 20 Sep 2022 12:50:23 +0100
Subject: [PATCH 10/13] Reword suggestion on using debug flags

---
 .../src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp   | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp
index bda7fd0926d..498094b0ef7 100644
--- a/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp
+++ b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp
@@ -6,11 +6,10 @@
 <overview>
 <p>The <code>WebView.setWebContentsDebuggingEnabled</code> method enables or disables the contents of any <code>WebView</code> in the application to be debugged.</p>
 
-<p>You should only enable debugging features during development. When you create a production build, you should disable it. If you enable debugging features, this can make your code vulnerable by adding entry points, or leaking sensitive information.
+<p>You should only enable debugging features during development. When you create a production build, you should disable it. If you enable debugging features, this can make your code vulnerable by adding entry points, or leaking sensitive information.</p>
 </overview>
 <recommendation>
-<p>Ensure that debugging features are not enabled in production builds. 
-If <code>WebView.setWebContentsDebuggingEnabled(true)</code> is used, ensure that it is guarded by a flag indicating that this is a debug build.</p>
+<p>Ensure that debugging features are not enabled in production builds, such as by guarding calls to <code>WebView.setWebContentsDebuggingEnabled(true)</code> by a flag that is only enabled in debug builds. 
 
 </recommendation>
 <example>

From eb3655da1c094ae3f193d44ffb30d963b9c71463 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Tue, 20 Sep 2022 14:48:21 +0100
Subject: [PATCH 11/13] Remove type check from the barrier predicate

---
 .../semmle/code/java/security/WebviewDubuggingEnabledQuery.qll  | 2 --
 1 file changed, 2 deletions(-)

diff --git a/java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll b/java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll
index 3b937766899..92071c2af1a 100644
--- a/java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll
+++ b/java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll
@@ -34,8 +34,6 @@ class WebviewDebugEnabledConfig extends DataFlow::Configuration {
   }
 
   override predicate isBarrier(DataFlow::Node node) {
-    not node.getType() instanceof BooleanType
-    or
     exists(Guard debug | isDebugCheck(debug) and debug.controls(node.asExpr().getBasicBlock(), _))
     or
     node.getEnclosingCallable().getDeclaringType() instanceof NonSecurityTestClass

From 2414239e50128413c88c49f431903db6f6321499 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Wed, 21 Sep 2022 16:36:20 +0100
Subject: [PATCH 12/13] Fix qhelp formatting

---
 java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp
index 498094b0ef7..d262d3b71c9 100644
--- a/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp
+++ b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp
@@ -9,7 +9,7 @@
 <p>You should only enable debugging features during development. When you create a production build, you should disable it. If you enable debugging features, this can make your code vulnerable by adding entry points, or leaking sensitive information.</p>
 </overview>
 <recommendation>
-<p>Ensure that debugging features are not enabled in production builds, such as by guarding calls to <code>WebView.setWebContentsDebuggingEnabled(true)</code> by a flag that is only enabled in debug builds. 
+<p>Ensure that debugging features are not enabled in production builds, such as by guarding calls to <code>WebView.setWebContentsDebuggingEnabled(true)</code> by a flag that is only enabled in debug builds. </p>
 
 </recommendation>
 <example>

From af41f2b90317b7734ef13fb6602b2b5d0c4f3ffc Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Mon, 26 Sep 2022 13:36:14 +0100
Subject: [PATCH 13/13] Remove 'here'.

---
 java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql
index efcc8666265..a8f7f81d832 100644
--- a/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql
+++ b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql
@@ -16,4 +16,4 @@ import DataFlow::PathGraph
 
 from WebviewDebugEnabledConfig conf, DataFlow::PathNode source, DataFlow::PathNode sink
 where conf.hasFlowPath(source, sink)
-select sink, source, sink, "Webview debugging is enabled here."
+select sink, source, sink, "Webview debugging is enabled."