Skip to content

Commit 4c01875

Browse files
egregius313egregius313
authored
Merge pull request #11283 from egregius313/egregius313/webview-setAllowContentAccess
Java: Android WebView Content Access Query
2 parents 54b3262 + 909b1d7 commit 4c01875

File tree

11 files changed

+243
-1
lines changed

11 files changed

+243
-1
lines changed

‎java/ql/lib/change-notes/2022-12-21-allowcontentaccessmethod.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added `AllowContentAccessMethod` to represent the `setAllowContentAccess` method of the `android.webkit.WebSettings` class.

‎java/ql/lib/semmle/code/java/frameworks/android/WebView.qll

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,14 @@ class WebViewSetWebViewClientMethod extends Method {
7878
}
7979
}
8080

81+
/** The method `setAllowContentAccess` of the class `android.webkit.WebSettings` */
82+
class AllowContentAccessMethod extends Method {
83+
AllowContentAccessMethod() {
84+
this.getDeclaringType() instanceof TypeWebSettings and
85+
this.hasName("setAllowContentAccess")
86+
}
87+
}
88+
8189
/** The method `shouldOverrideUrlLoading` of the class `android.webkit.WebViewClient`. */
8290
class ShouldOverrideUrlLoading extends Method {
8391
ShouldOverrideUrlLoading() {

‎java/ql/src/Security/CWE/CWE-200/AndroidWebViewSettingsAllowsContentAccess.qhelp

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>Android can provide access to content providers within a WebView using
7+
the <code>setAllowContentAccess</code> setting.</p>
8+
9+
<p>Allowing access to content providers via <code>content://</code> URLs
10+
may allow JavaScript to access protected content.</p>
11+
</overview>
12+
13+
<recommendation>
14+
<p>
15+
If your app does not require access to the <code>content://</code> URL
16+
functionality, you should explicitly disable the setting by
17+
calling <code>setAllowContentAccess(false)</code> on the settings of the
18+
WebView.
19+
</p>
20+
</recommendation>
21+
22+
<example>
23+
<p>In the following (bad) example, access to <code>content://</code> URLs is explicitly allowed.</p>
24+
25+
<sample src="ContentAccessEnabled.java"/>
26+
27+
<p>In the following (good) example, access to <code>content://</code> URLs is explicitly denied.</p>
28+
29+
<sample src="ContentAccessDisabled.java"/>
30+
</example>
31+
32+
<references>
33+
<li>
34+
Android Documentation: <a href="https://developer.android.com/reference/android/webkit/WebSettings#setAllowContentAccess(boolean)">setAllowContentAccess</a>.
35+
</li>
36+
</references>
37+
38+
</qhelp>

‎java/ql/src/Security/CWE/CWE-200/AndroidWebViewSettingsAllowsContentAccess.ql

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/**
2+
* @name Android WebView settings allows access to content links
3+
* @id java/android/websettings-allow-content-access
4+
* @description Access to content providers in a WebView can allow access to protected information by loading content:// links.
5+
* @kind problem
6+
* @problem.severity warning
7+
* @precision medium
8+
* @security-severity 6.5
9+
* @tags security
10+
* external/cwe/cwe-200
11+
*/
12+
13+
import java
14+
import semmle.code.java.dataflow.TaintTracking
15+
import semmle.code.java.frameworks.android.WebView
16+
17+
/** Represents `android.webkit.WebView` and its subclasses. */
18+
private class TypeWebViewOrSubclass extends RefType {
19+
TypeWebViewOrSubclass() { this.getASupertype*() instanceof TypeWebView }
20+
}
21+
22+
/**
23+
* A method access to a getter method which is private.
24+
*
25+
* In Kotlin, member accesses are translated to getter methods.
26+
*/
27+
private class PrivateGetterMethodAccess extends MethodAccess {
28+
PrivateGetterMethodAccess() {
29+
this.getMethod() instanceof GetterMethod and
30+
this.getMethod().isPrivate()
31+
}
32+
}
33+
34+
/** A source for `android.webkit.WebView` objects. */
35+
class WebViewSource extends DataFlow::Node {
36+
WebViewSource() {
37+
this.getType() instanceof TypeWebViewOrSubclass and
38+
// To reduce duplicate results, we only consider WebView objects from
39+
// constructor and method calls, or method accesses which are cast to WebView.
40+
(
41+
this.asExpr() instanceof ClassInstanceExpr or
42+
this.asExpr() instanceof MethodAccess or
43+
this.asExpr().(CastExpr).getAChildExpr() instanceof MethodAccess
44+
) and
45+
// Avoid duplicate results from Kotlin member accesses.
46+
not this.asExpr() instanceof PrivateGetterMethodAccess
47+
}
48+
}
49+
50+
/**
51+
* A sink representing a call to `android.webkit.WebSettings.setAllowContentAccess` that
52+
* disables content access.
53+
*/
54+
class WebSettingsDisallowContentAccessSink extends DataFlow::Node {
55+
WebSettingsDisallowContentAccessSink() {
56+
exists(MethodAccess ma |
57+
ma.getQualifier() = this.asExpr() and
58+
ma.getMethod() instanceof AllowContentAccessMethod and
59+
ma.getArgument(0).(CompileTimeConstantExpr).getBooleanValue() = false
60+
)
61+
}
62+
}
63+
64+
class WebViewDisallowContentAccessConfiguration extends TaintTracking::Configuration {
65+
WebViewDisallowContentAccessConfiguration() { this = "WebViewDisallowContentAccessConfiguration" }
66+
67+
override predicate isSource(DataFlow::Node node) { node instanceof WebViewSource }
68+
69+
/**
70+
* Holds if the step from `node1` to `node2` is a dataflow step that gets the `WebSettings` object
71+
* from the `getSettings` method of a `WebView` object.
72+
*
73+
* This step is only valid when `state1` is empty and `state2` indicates that the `WebSettings` object
74+
* has been accessed.
75+
*/
76+
override predicate isAdditionalTaintStep(
77+
DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
78+
DataFlow::FlowState state2
79+
) {
80+
state1 instanceof DataFlow::FlowStateEmpty and
81+
state2 = "WebSettings" and
82+
// settings = webView.getSettings()
83+
// ^node2 = ^node1
84+
exists(MethodAccess ma |
85+
ma = node2.asExpr() and
86+
ma.getQualifier() = node1.asExpr() and
87+
ma.getMethod() instanceof WebViewGetSettingsMethod
88+
)
89+
}
90+
91+
override predicate isSink(DataFlow::Node node, DataFlow::FlowState state) {
92+
state = "WebSettings" and
93+
node instanceof WebSettingsDisallowContentAccessSink
94+
}
95+
}
96+
97+
from Expr e
98+
where
99+
// explicit: setAllowContentAccess(true)
100+
exists(MethodAccess ma |
101+
ma = e and
102+
ma.getMethod() instanceof AllowContentAccessMethod and
103+
ma.getArgument(0).(CompileTimeConstantExpr).getBooleanValue() = true
104+
)
105+
or
106+
// implicit: no setAllowContentAccess(false)
107+
exists(WebViewSource source |
108+
source.asExpr() = e and
109+
not any(WebViewDisallowContentAccessConfiguration cfg).hasFlow(source, _)
110+
)
111+
select e,
112+
"Sensitive information may be exposed via a malicious link due to access to content:// links being allowed in this WebView."

‎java/ql/src/Security/CWE/CWE-200/ContentAccessDisabled.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
WebSettings settings = webview.getSettings();
2+
3+
settings.setAllowContentAccess(false);

‎java/ql/src/Security/CWE/CWE-200/ContentAccessEnabled.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
WebSettings settings = webview.getSettings();
2+
3+
settings.setAllowContentAccess(true);

‎java/ql/src/change-notes/2022-12-21-android-allowcontentaccess-query.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* Added a new query `java/android/websettings-allow-content-access` to detect Android WebViews which do not disable access to `content://` urls.

‎java/ql/test/query-tests/security/CWE-200/semmle/tests/WebViewContentAccess.expected

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
| WebViewContentAccess.java:15:9:15:57 | setAllowContentAccess(...) | Sensitive information may be exposed via a malicious link due to access to content:// links being allowed in this WebView. |
2+
| WebViewContentAccess.java:38:9:38:55 | setAllowContentAccess(...) | Sensitive information may be exposed via a malicious link due to access to content:// links being allowed in this WebView. |
3+
| WebViewContentAccess.java:41:25:41:49 | (...)... | Sensitive information may be exposed via a malicious link due to access to content:// links being allowed in this WebView. |
4+
| WebViewContentAccess.java:43:9:43:44 | setAllowContentAccess(...) | Sensitive information may be exposed via a malicious link due to access to content:// links being allowed in this WebView. |
5+
| WebViewContentAccess.java:46:25:46:41 | new WebView(...) | Sensitive information may be exposed via a malicious link due to access to content:// links being allowed in this WebView. |
6+
| WebViewContentAccess.java:48:9:48:44 | setAllowContentAccess(...) | Sensitive information may be exposed via a malicious link due to access to content:// links being allowed in this WebView. |
7+
| WebViewContentAccess.java:51:25:51:44 | getAWebView(...) | Sensitive information may be exposed via a malicious link due to access to content:// links being allowed in this WebView. |
8+
| WebViewContentAccess.java:53:9:53:44 | setAllowContentAccess(...) | Sensitive information may be exposed via a malicious link due to access to content:// links being allowed in this WebView. |
9+
| WebViewContentAccess.java:55:29:55:48 | getAWebView(...) | Sensitive information may be exposed via a malicious link due to access to content:// links being allowed in this WebView. |
10+
| WebViewContentAccess.java:57:25:57:44 | getAWebView(...) | Sensitive information may be exposed via a malicious link due to access to content:// links being allowed in this WebView. |

‎java/ql/test/query-tests/security/CWE-200/semmle/tests/WebViewContentAccess.java

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
package com.example.test;
2+
3+
import android.app.Activity;
4+
5+
import android.webkit.WebView;
6+
import android.webkit.WebSettings;
7+
8+
/** Helper class to mock a method which returns a `WebView` */
9+
interface WebViewGetter {
10+
WebView getAWebView();
11+
}
12+
13+
public class WebViewContentAccess extends Activity {
14+
void enableContentAccess(WebView webview) {
15+
webview.getSettings().setAllowContentAccess(true);
16+
}
17+
18+
void disableContentAccess(WebView webview) {
19+
webview.getSettings().setAllowContentAccess(false);
20+
}
21+
22+
void configureWebViewSafe(WebView view, WebViewGetter getter) {
23+
WebSettings settings = view.getSettings();
24+
25+
settings.setAllowContentAccess(false);
26+
27+
WebView view2 = (WebView) findViewById(0);
28+
settings = view2.getSettings();
29+
30+
settings.setAllowContentAccess(false);
31+
32+
disableContentAccess(getter.getAWebView());
33+
}
34+
35+
void configureWebViewUnsafe(WebView view1, WebViewGetter getter) {
36+
WebSettings settings;
37+
38+
view1.getSettings().setAllowContentAccess(true);
39+
40+
// Cast expression
41+
WebView view2 = (WebView) findViewById(0);
42+
settings = view2.getSettings();
43+
settings.setAllowContentAccess(true);
44+
45+
// Constructor
46+
WebView view3 = new WebView(this);
47+
settings = view3.getSettings();
48+
settings.setAllowContentAccess(true);
49+
50+
// Method access
51+
WebView view4 = getter.getAWebView();
52+
settings = view4.getSettings();
53+
settings.setAllowContentAccess(true);
54+
55+
enableContentAccess(getter.getAWebView());
56+
57+
WebView view5 = getter.getAWebView();
58+
}
59+
}

‎java/ql/test/query-tests/security/CWE-200/semmle/tests/WebViewContentAccess.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE/CWE-200/AndroidWebViewSettingsAllowsContentAccess.ql

0 commit comments

Comments
 (0)