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..92071c2af1a --- /dev/null +++ b/java/ql/lib/semmle/code/java/security/WebviewDubuggingEnabledQuery.qll @@ -0,0 +1,41 @@ +/** 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%", "%test%"]) and + subex.getParent*() = ex + | + subex.(VarAccess).getVariable().getName() = debug + or + subex.(MethodAccess).getMethod().hasName("getProperty") and + subex.(MethodAccess).getAnArgument().(CompileTimeConstantExpr).getStringValue() = debug + ) +} + +/** A 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) { + 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.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..d262d3b71c9 --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.qhelp @@ -0,0 +1,36 @@ + + + + +

The WebView.setWebContentsDebuggingEnabled method enables or disables the contents of any WebView in the application to be debugged.

+ +

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.

+
+ +

Ensure that debugging features are not enabled in production builds, such as by guarding calls to WebView.setWebContentsDebuggingEnabled(true) by a flag that is only enabled in debug builds.

+ +
+ + +

In the first (bad) example, WebView debugging is always enabled. +whereas the GOOD case only enables it if the android:debuggable attribute is set to true.

+ + + +
+ + +
  • + Android Developers: + setWebContentsDebuggingEnabled. +
  • + +
  • + Android Developers: + Remote debugging WebViews. +
  • + +
    +
    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..a8f7f81d832 --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-489/WebviewDebuggingEnabled.ql @@ -0,0 +1,19 @@ +/** + * @name Android Webview debugging enabled + * @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 + * @id java/android/webview-debugging-enabled + * @tags security + * external/cwe/cwe-489 + * @precision high + */ + +import java +import semmle.code.java.security.WebviewDubuggingEnabledQuery +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." 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 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/debuggable-attribute/options b/java/ql/test/query-tests/security/CWE-489/debuggable-attribute/options new file mode 100644 index 00000000000..33cdc1ea940 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-489/debuggable-attribute/options @@ -0,0 +1 @@ +//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/google-android-9.0.0 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/options b/java/ql/test/query-tests/security/CWE-489/webview-debugging/options similarity index 68% rename from java/ql/test/query-tests/security/CWE-489/options rename to java/ql/test/query-tests/security/CWE-489/webview-debugging/options index dacd3cb21df..3042f155024 100644 --- a/java/ql/test/query-tests/security/CWE-489/options +++ b/java/ql/test/query-tests/security/CWE-489/webview-debugging/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 \ No newline at end of file