From 28ad9d00fb8bcb43dd860178a932a34756955d7c Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Tue, 3 Jan 2023 15:17:07 -0500 Subject: [PATCH 1/2] Merge both setAllowContentAccess queries into one query Previously, the query to detect whether or not access to `content://` links was done using two queries. Now they can be merged into one query --- .../AndroidWebViewSettingsContentAccess.ql | 21 ------------------- ...WebViewSettingsPermitsContentAccess.qhelp} | 0 ...roidWebViewSettingsPermitsContentAccess.ql | 18 +++++++++++++--- .../tests/WebViewContentAccess.expected | 5 +++++ .../semmle/tests/WebViewContentAccess.qlref | 2 +- .../WebViewContentAccessDataFlow.expected | 5 ----- .../tests/WebViewContentAccessDataFlow.qlref | 1 - 7 files changed, 21 insertions(+), 31 deletions(-) delete mode 100644 java/ql/src/Security/CWE/CWE-200/AndroidWebViewSettingsContentAccess.ql rename java/ql/src/Security/CWE/CWE-200/{AndroidWebViewSettingsContentAccess.qhelp => AndroidWebViewSettingsPermitsContentAccess.qhelp} (100%) delete mode 100644 java/ql/test/query-tests/security/CWE-200/semmle/tests/WebViewContentAccessDataFlow.expected delete mode 100644 java/ql/test/query-tests/security/CWE-200/semmle/tests/WebViewContentAccessDataFlow.qlref diff --git a/java/ql/src/Security/CWE/CWE-200/AndroidWebViewSettingsContentAccess.ql b/java/ql/src/Security/CWE/CWE-200/AndroidWebViewSettingsContentAccess.ql deleted file mode 100644 index 1657f4177543..000000000000 --- a/java/ql/src/Security/CWE/CWE-200/AndroidWebViewSettingsContentAccess.ql +++ /dev/null @@ -1,21 +0,0 @@ -/** - * @name Android WebSettings content access - * @description Access to content providers in a WebView can enable JavaScript to access protected information. - * @kind problem - * @id java/android/websettings-content-access - * @problem.severity warning - * @security-severity 6.5 - * @precision medium - * @tags security - * external/cwe/cwe-200 - */ - -import java -import semmle.code.java.frameworks.android.WebView - -from MethodAccess ma -where - ma.getMethod() instanceof AllowContentAccessMethod and - ma.getArgument(0).(CompileTimeConstantExpr).getBooleanValue() = true -select ma, - "Sensitive information may be exposed via a malicious link due to access of content:// links being permitted." diff --git a/java/ql/src/Security/CWE/CWE-200/AndroidWebViewSettingsContentAccess.qhelp b/java/ql/src/Security/CWE/CWE-200/AndroidWebViewSettingsPermitsContentAccess.qhelp similarity index 100% rename from java/ql/src/Security/CWE/CWE-200/AndroidWebViewSettingsContentAccess.qhelp rename to java/ql/src/Security/CWE/CWE-200/AndroidWebViewSettingsPermitsContentAccess.qhelp diff --git a/java/ql/src/Security/CWE/CWE-200/AndroidWebViewSettingsPermitsContentAccess.ql b/java/ql/src/Security/CWE/CWE-200/AndroidWebViewSettingsPermitsContentAccess.ql index 509b062c5955..7ccf23cc3ae2 100644 --- a/java/ql/src/Security/CWE/CWE-200/AndroidWebViewSettingsPermitsContentAccess.ql +++ b/java/ql/src/Security/CWE/CWE-200/AndroidWebViewSettingsPermitsContentAccess.ql @@ -94,7 +94,19 @@ class WebViewDisallowContentAccessConfiguration extends TaintTracking::Configura } } -from WebViewSource source -where not any(WebViewDisallowContentAccessConfiguration cfg).hasFlow(source, _) -select source, +from Expr e +where + // explicit: setAllowContentAccess(true) + exists(MethodAccess ma | + ma = e and + ma.getMethod() instanceof AllowContentAccessMethod and + ma.getArgument(0).(CompileTimeConstantExpr).getBooleanValue() = true + ) + or + // implicit: no setAllowContentAccess(false) + exists(WebViewSource source | + source.asExpr() = e and + not any(WebViewDisallowContentAccessConfiguration cfg).hasFlow(source, _) + ) +select e, "Sensitive information may be exposed via a malicious link due to access of content:// links being permitted." diff --git a/java/ql/test/query-tests/security/CWE-200/semmle/tests/WebViewContentAccess.expected b/java/ql/test/query-tests/security/CWE-200/semmle/tests/WebViewContentAccess.expected index dc904ca6cd74..317f847279f8 100644 --- a/java/ql/test/query-tests/security/CWE-200/semmle/tests/WebViewContentAccess.expected +++ b/java/ql/test/query-tests/security/CWE-200/semmle/tests/WebViewContentAccess.expected @@ -1,5 +1,10 @@ | WebViewContentAccess.java:15:9:15:57 | setAllowContentAccess(...) | Sensitive information may be exposed via a malicious link due to access of content:// links being permitted. | | WebViewContentAccess.java:38:9:38:55 | setAllowContentAccess(...) | Sensitive information may be exposed via a malicious link due to access of content:// links being permitted. | +| WebViewContentAccess.java:41:25:41:49 | (...)... | Sensitive information may be exposed via a malicious link due to access of content:// links being permitted. | | WebViewContentAccess.java:43:9:43:44 | setAllowContentAccess(...) | Sensitive information may be exposed via a malicious link due to access of content:// links being permitted. | +| WebViewContentAccess.java:46:25:46:41 | new WebView(...) | Sensitive information may be exposed via a malicious link due to access of content:// links being permitted. | | WebViewContentAccess.java:48:9:48:44 | setAllowContentAccess(...) | Sensitive information may be exposed via a malicious link due to access of content:// links being permitted. | +| WebViewContentAccess.java:51:25:51:44 | getAWebView(...) | Sensitive information may be exposed via a malicious link due to access of content:// links being permitted. | | WebViewContentAccess.java:53:9:53:44 | setAllowContentAccess(...) | Sensitive information may be exposed via a malicious link due to access of content:// links being permitted. | +| WebViewContentAccess.java:55:29:55:48 | getAWebView(...) | Sensitive information may be exposed via a malicious link due to access of content:// links being permitted. | +| WebViewContentAccess.java:57:25:57:44 | getAWebView(...) | Sensitive information may be exposed via a malicious link due to access of content:// links being permitted. | diff --git a/java/ql/test/query-tests/security/CWE-200/semmle/tests/WebViewContentAccess.qlref b/java/ql/test/query-tests/security/CWE-200/semmle/tests/WebViewContentAccess.qlref index f907dcc57556..8ea25a487de8 100644 --- a/java/ql/test/query-tests/security/CWE-200/semmle/tests/WebViewContentAccess.qlref +++ b/java/ql/test/query-tests/security/CWE-200/semmle/tests/WebViewContentAccess.qlref @@ -1 +1 @@ -Security/CWE/CWE-200/AndroidWebViewSettingsContentAccess.ql +Security/CWE/CWE-200/AndroidWebViewSettingsPermitsContentAccess.ql \ No newline at end of file diff --git a/java/ql/test/query-tests/security/CWE-200/semmle/tests/WebViewContentAccessDataFlow.expected b/java/ql/test/query-tests/security/CWE-200/semmle/tests/WebViewContentAccessDataFlow.expected deleted file mode 100644 index 6b0c6c526258..000000000000 --- a/java/ql/test/query-tests/security/CWE-200/semmle/tests/WebViewContentAccessDataFlow.expected +++ /dev/null @@ -1,5 +0,0 @@ -| WebViewContentAccess.java:41:25:41:49 | (...)... | Sensitive information may be exposed via a malicious link due to access of content:// links being permitted. | -| WebViewContentAccess.java:46:25:46:41 | new WebView(...) | Sensitive information may be exposed via a malicious link due to access of content:// links being permitted. | -| WebViewContentAccess.java:51:25:51:44 | getAWebView(...) | Sensitive information may be exposed via a malicious link due to access of content:// links being permitted. | -| WebViewContentAccess.java:55:29:55:48 | getAWebView(...) | Sensitive information may be exposed via a malicious link due to access of content:// links being permitted. | -| WebViewContentAccess.java:57:25:57:44 | getAWebView(...) | Sensitive information may be exposed via a malicious link due to access of content:// links being permitted. | diff --git a/java/ql/test/query-tests/security/CWE-200/semmle/tests/WebViewContentAccessDataFlow.qlref b/java/ql/test/query-tests/security/CWE-200/semmle/tests/WebViewContentAccessDataFlow.qlref deleted file mode 100644 index 8ea25a487de8..000000000000 --- a/java/ql/test/query-tests/security/CWE-200/semmle/tests/WebViewContentAccessDataFlow.qlref +++ /dev/null @@ -1 +0,0 @@ -Security/CWE/CWE-200/AndroidWebViewSettingsPermitsContentAccess.ql \ No newline at end of file From 81df89f93e29a28a686c4a598ac74cd4d6e0ea95 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Tue, 3 Jan 2023 15:19:26 -0500 Subject: [PATCH 2/2] Use proper @id in changenote --- .../change-notes/2022-12-21-android-allowcontentaccess-query.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/src/change-notes/2022-12-21-android-allowcontentaccess-query.md b/java/ql/src/change-notes/2022-12-21-android-allowcontentaccess-query.md index 008da665b579..854da87eb54c 100644 --- a/java/ql/src/change-notes/2022-12-21-android-allowcontentaccess-query.md +++ b/java/ql/src/change-notes/2022-12-21-android-allowcontentaccess-query.md @@ -1,4 +1,4 @@ --- category: newQuery --- -* Added a new query `java/android/websettings-content-access` to detect Android WebViews which do not disable access to `content://` urls. +* Added a new query `java/android/websettings-permit-contentacces` to detect Android WebViews which do not disable access to `content://` urls.