Skip to content

Commit 3e03db1

Browse files
smowtonsmowton
authored
Merge pull request #4483 from smowton/smowton/admin/droid-webview-pr-rebase
Rebase of #3706
2 parents 7942d73 + 5a480bf commit 3e03db1

File tree

21 files changed

+9966
-0
lines changed

21 files changed

+9966
-0
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-749/UnsafeAndroidAccess.java‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
public class UnsafeAndroidAccess extends Activity {
2+
public void onCreate(Bundle savedInstanceState) {
3+
super.onCreate(savedInstanceState);
4+
setContentView(R.layout.webview);
5+
6+
// BAD: Have both JavaScript and cross-origin resource access enabled in webview while
7+
// taking remote user inputs
8+
{
9+
WebView wv = (WebView) findViewById(R.id.my_webview);
10+
WebSettings webSettings = wv.getSettings();
11+
12+
webSettings.setJavaScriptEnabled(true);
13+
webSettings.setAllowUniversalAccessFromFileURLs(true);
14+
15+
wv.setWebViewClient(new WebViewClient() {
16+
@Override
17+
public boolean shouldOverrideUrlLoading(WebView view, String url) {
18+
view.loadUrl(url);
19+
return true;
20+
}
21+
});
22+
23+
String thisUrl = getIntent().getExtras().getString("url"); // dangerous remote input from the intent's Bundle of extras
24+
wv.loadUrl(thisUrl);
25+
}
26+
27+
// BAD: Have both JavaScript and cross-origin resource access enabled in webview while
28+
// taking remote user inputs
29+
{
30+
WebView wv = (WebView) findViewById(R.id.my_webview);
31+
WebSettings webSettings = wv.getSettings();
32+
33+
webSettings.setJavaScriptEnabled(true);
34+
webSettings.setAllowUniversalAccessFromFileURLs(true);
35+
36+
wv.setWebViewClient(new WebViewClient() {
37+
@Override
38+
public boolean shouldOverrideUrlLoading(WebView view, String url) {
39+
view.loadUrl(url);
40+
return true;
41+
}
42+
});
43+
44+
String thisUrl = getIntent().getStringExtra("url"); //dangerous remote input from intent extra
45+
wv.loadUrl(thisUrl);
46+
}
47+
48+
// GOOD: Have JavaScript and cross-origin resource access disabled by default on modern Android (Jellybean+) while taking remote user inputs
49+
{
50+
WebView wv = (WebView) findViewById(-1);
51+
WebSettings webSettings = wv.getSettings();
52+
53+
wv.setWebViewClient(new WebViewClient() {
54+
@Override
55+
public boolean shouldOverrideUrlLoading(WebView view, String url) {
56+
view.loadUrl(url);
57+
return true;
58+
}
59+
});
60+
61+
String thisUrl = getIntent().getExtras().getString("url"); // remote input
62+
wv.loadUrl(thisUrl);
63+
}
64+
65+
// GOOD: Have JavaScript enabled in webview but remote user input is not allowed
66+
{
67+
WebView wv = (WebView) findViewById(-1);
68+
WebSettings webSettings = wv.getSettings();
69+
70+
webSettings.setJavaScriptEnabled(true);
71+
72+
wv.setWebViewClient(new WebViewClient() {
73+
@Override
74+
public boolean shouldOverrideUrlLoading(WebView view, String url) {
75+
view.loadUrl(url);
76+
return true;
77+
}
78+
});
79+
80+
wv.loadUrl("https://www.mycorp.com");
81+
}
82+
}
83+
}

‎java/ql/src/experimental/Security/CWE/CWE-749/UnsafeAndroidAccess.qhelp‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<overview>
5+
<p>Android WebViews that allow loading URLs controlled by external inputs and whose JavaScript interface is enabled are potentially vulnerable to cross-site scripting and sensitive resource disclosure attacks.</p>
6+
<p>A <code>WebView</code> whose <code>WebSettings</code> object has <code>setAllowFileAccessFromFileURLs(true)</code> or <code>setAllowUniversalAccessFromFileURLs(true)</code> called must not load any untrusted web content.</p>
7+
<p>Enabling these settings allows malicious scripts loaded in a <code>file://</code> context to launch cross-site scripting attacks, either accessing arbitrary local files including WebView cookies, session tokens, private app data or even credentials used on arbitrary web sites.</p>
8+
<p>This query detects the following two scenarios:</p>
9+
<ol>
10+
<li>Vulnerability introduced by WebViews with JavaScript enabled and remote inputs allowed.</li>
11+
<li>A more severe vulnerability when allowing cross-origin resource access is also enabled. The setting was deprecated in API level 30 (Android 11), but most devices are still affected, especially given that some Android phones are updated slowly or no longer updated at all.</li>
12+
</ol>
13+
</overview>
14+
15+
<recommendation>
16+
<p>Only allow trusted web content to be displayed in WebViews when JavaScript is enabled. Disallow cross-origin resource access in WebSetting to reduce the attack surface.</p>
17+
</recommendation>
18+
19+
<example>
20+
<p>The following example shows both 'BAD' and 'GOOD' configurations. In the 'BAD' configuration, setting is enabled and JavaScript is enabled while URLs are loaded from externally controlled inputs. In the 'GOOD' configuration, JavaScript is disabled or only trusted web content is allowed to be loaded.</p>
21+
<sample src="UnsafeAndroidAccess.java" />
22+
</example>
23+
24+
<references>
25+
<li>
26+
<a href="https://cwe.mitre.org/data/definitions/749.html">CWE-749</a>
27+
<a href="https://support.google.com/faqs/answer/7668153?hl=en">Fixing a File-based XSS Vulnerability</a>
28+
<a href="https://github.com/OWASP/owasp-mstg/blob/master/Document/0x05h-Testing-Platform-Interaction.md">OWASP - Testing WebView Protocol Handlers (MSTG-PLATFORM-5 and MSTG-PLATFORM-6)</a>
29+
</li>
30+
</references>
31+
</qhelp>

‎java/ql/src/experimental/Security/CWE/CWE-749/UnsafeAndroidAccess.ql‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
/**
2+
* @name Unsafe resource fetching in Android webview
3+
* @id java/android/unsafe-android-webview-fetch
4+
* @description JavaScript rendered inside WebViews can access any protected application file and web resource from any origin
5+
* @kind path-problem
6+
* @tags security
7+
* external/cwe/cwe-749
8+
* external/cwe/cwe-079
9+
*/
10+
11+
import java
12+
import semmle.code.java.frameworks.android.Intent
13+
import semmle.code.java.frameworks.android.WebView
14+
import semmle.code.java.dataflow.FlowSources
15+
import DataFlow::PathGraph
16+
17+
/**
18+
* Methods allowing any-local-file and cross-origin access in the WebSettings class
19+
*/
20+
class CrossOriginAccessMethod extends Method {
21+
CrossOriginAccessMethod() {
22+
this.getDeclaringType() instanceof TypeWebSettings and
23+
(
24+
this.hasName("setAllowUniversalAccessFromFileURLs") or
25+
this.hasName("setAllowFileAccessFromFileURLs")
26+
)
27+
}
28+
}
29+
30+
/**
31+
* `setJavaScriptEnabled` method for the webview
32+
*/
33+
class AllowJavaScriptMethod extends Method {
34+
AllowJavaScriptMethod() {
35+
this.getDeclaringType() instanceof TypeWebSettings and
36+
this.hasName("setJavaScriptEnabled")
37+
}
38+
}
39+
40+
/**
41+
* Holds if a call to `v.setJavaScriptEnabled(true)` exists
42+
*/
43+
predicate isJSEnabled(Variable v) {
44+
exists(MethodAccess jsa |
45+
v.getAnAccess() = jsa.getQualifier() and
46+
jsa.getMethod() instanceof AllowJavaScriptMethod and
47+
jsa.getArgument(0).(BooleanLiteral).getBooleanValue() = true
48+
)
49+
}
50+
51+
/**
52+
* Fetch URL method call on the `android.webkit.WebView` object
53+
*/
54+
class FetchResourceMethodAccess extends MethodAccess {
55+
FetchResourceMethodAccess() {
56+
this.getMethod().getDeclaringType() instanceof TypeWebView and
57+
this.getMethod().hasName(["loadUrl", "postUrl"])
58+
}
59+
}
60+
61+
/**
62+
* Method access to external inputs of `android.content.Intent` object
63+
*/
64+
class IntentGetExtraMethodAccess extends MethodAccess {
65+
IntentGetExtraMethodAccess() {
66+
this.getMethod().getName().regexpMatch("get\\w+Extra") and
67+
this.getMethod().getDeclaringType() instanceof TypeIntent
68+
or
69+
this.getMethod().getName().regexpMatch("get\\w+") and
70+
this.getQualifier().(MethodAccess).getMethod().hasName("getExtras") and
71+
this.getQualifier().(MethodAccess).getMethod().getDeclaringType() instanceof TypeIntent
72+
}
73+
}
74+
75+
/**
76+
* Source of fetching URLs from intent extras
77+
*/
78+
class UntrustedResourceSource extends DataFlow::ExprNode {
79+
UntrustedResourceSource() { this.asExpr() instanceof IntentGetExtraMethodAccess }
80+
}
81+
82+
/**
83+
* Holds if `ma` loads URL `sink`
84+
*/
85+
predicate fetchResource(FetchResourceMethodAccess ma, Expr sink) { sink = ma.getArgument(0) }
86+
87+
/**
88+
* A URL argument to a `loadUrl` or `postUrl` call, considered as a sink.
89+
*/
90+
class UrlResourceSink extends DataFlow::ExprNode {
91+
UrlResourceSink() { fetchResource(_, this.getExpr()) }
92+
93+
/** Gets the fetch method that fetches this sink URL. */
94+
FetchResourceMethodAccess getMethodAccess() { fetchResource(result, this.getExpr()) }
95+
96+
/**
97+
* Holds if cross-origin access is enabled for this resource fetch.
98+
*
99+
* Specifically this looks for code like
100+
* `webView.getSettings().setAllow[File|Universal]AccessFromFileURLs(true);`
101+
*/
102+
predicate crossOriginAccessEnabled() {
103+
exists(MethodAccess ma, MethodAccess getSettingsMa |
104+
ma.getMethod() instanceof CrossOriginAccessMethod and
105+
ma.getArgument(0).(BooleanLiteral).getBooleanValue() = true and
106+
ma.getQualifier().(VarAccess).getVariable().getAnAssignedValue() = getSettingsMa and
107+
getSettingsMa.getMethod() instanceof WebViewGetSettingsMethod and
108+
getSettingsMa.getQualifier().(VarAccess).getVariable().getAnAccess() =
109+
getMethodAccess().getQualifier()
110+
)
111+
}
112+
113+
/**
114+
* Returns a description of this vulnerability, assuming Javascript is enabled and
115+
* the fetched URL is attacker-controlled.
116+
*/
117+
string getSinkType() {
118+
if crossOriginAccessEnabled()
119+
then result = "user input vulnerable to cross-origin and sensitive resource disclosure attacks"
120+
else result = "user input vulnerable to XSS attacks"
121+
}
122+
}
123+
124+
/**
125+
* Taint configuration tracking flow from untrusted inputs to `loadUrl` or `postUrl` calls.
126+
*/
127+
class FetchUntrustedResourceConfiguration extends TaintTracking::Configuration {
128+
FetchUntrustedResourceConfiguration() { this = "FetchUntrustedResourceConfiguration" }
129+
130+
override predicate isSource(DataFlow::Node source) { source instanceof UntrustedResourceSource }
131+
132+
override predicate isSink(DataFlow::Node sink) {
133+
sink instanceof UrlResourceSink and
134+
exists(VarAccess webviewVa, MethodAccess getSettingsMa, Variable v |
135+
sink.(UrlResourceSink).getMethodAccess().getQualifier() = webviewVa and
136+
getSettingsMa.getMethod() instanceof WebViewGetSettingsMethod and
137+
webviewVa.getVariable().getAnAccess() = getSettingsMa.getQualifier() and
138+
v.getAnAssignedValue() = getSettingsMa and
139+
isJSEnabled(v)
140+
)
141+
}
142+
}
143+
144+
from DataFlow::PathNode source, DataFlow::PathNode sink, FetchUntrustedResourceConfiguration conf
145+
where conf.hasFlowPath(source, sink)
146+
select sink.getNode().(UrlResourceSink).getMethodAccess(), source, sink,
147+
"Unsafe resource fetching in Android webview due to $@.", source.getNode(),
148+
sink.getNode().(UrlResourceSink).getSinkType()

‎java/ql/test/experimental/query-tests/security/CWE-749/UnsafeAndroidAccess.expected‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
| UnsafeAndroidAccess.java:30:3:30:21 | loadUrl(...) | UnsafeAndroidAccess.java:29:20:29:59 | getString(...) : String | UnsafeAndroidAccess.java:30:14:30:20 | thisUrl | Unsafe resource fetching in Android webview due to $@. | UnsafeAndroidAccess.java:29:20:29:59 | getString(...) | user input vulnerable to cross-origin and sensitive resource disclosure attacks |
2+
| UnsafeAndroidAccess.java:53:3:53:21 | loadUrl(...) | UnsafeAndroidAccess.java:52:20:52:52 | getStringExtra(...) : String | UnsafeAndroidAccess.java:53:14:53:20 | thisUrl | Unsafe resource fetching in Android webview due to $@. | UnsafeAndroidAccess.java:52:20:52:52 | getStringExtra(...) | user input vulnerable to cross-origin and sensitive resource disclosure attacks |
3+
| UnsafeAndroidAccess.java:95:3:95:21 | loadUrl(...) | UnsafeAndroidAccess.java:94:20:94:52 | getStringExtra(...) : String | UnsafeAndroidAccess.java:95:14:95:20 | thisUrl | Unsafe resource fetching in Android webview due to $@. | UnsafeAndroidAccess.java:94:20:94:52 | getStringExtra(...) | user input vulnerable to XSS attacks |

‎java/ql/test/experimental/query-tests/security/CWE-749/UnsafeAndroidAccess.java‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
import android.app.Activity;
2+
3+
import android.os.Bundle;
4+
5+
import android.webkit.WebSettings;
6+
import android.webkit.WebView;
7+
import android.webkit.WebViewClient;
8+
9+
public class UnsafeAndroidAccess extends Activity {
10+
//Test onCreate with both JavaScript and cross-origin resource access enabled while taking remote user inputs from bundle extras
11+
public void testOnCreate1(Bundle savedInstanceState) {
12+
super.onCreate(savedInstanceState);
13+
setContentView(-1);
14+
15+
WebView wv = (WebView) findViewById(-1);
16+
WebSettings webSettings = wv.getSettings();
17+
18+
webSettings.setJavaScriptEnabled(true);
19+
webSettings.setAllowFileAccessFromFileURLs(true);
20+
21+
wv.setWebViewClient(new WebViewClient() {
22+
@Override
23+
public boolean shouldOverrideUrlLoading(WebView view, String url) {
24+
view.loadUrl(url);
25+
return true;
26+
}
27+
});
28+
29+
String thisUrl = getIntent().getExtras().getString("url");
30+
wv.loadUrl(thisUrl);
31+
}
32+
33+
//Test onCreate with both JavaScript and cross-origin resource access enabled while taking remote user inputs from string extra
34+
public void testOnCreate2(Bundle savedInstanceState) {
35+
super.onCreate(savedInstanceState);
36+
setContentView(-1);
37+
38+
WebView wv = (WebView) findViewById(-1);
39+
WebSettings webSettings = wv.getSettings();
40+
41+
webSettings.setJavaScriptEnabled(true);
42+
webSettings.setAllowFileAccessFromFileURLs(true);
43+
44+
wv.setWebViewClient(new WebViewClient() {
45+
@Override
46+
public boolean shouldOverrideUrlLoading(WebView view, String url) {
47+
view.loadUrl(url);
48+
return true;
49+
}
50+
});
51+
52+
String thisUrl = getIntent().getStringExtra("url");
53+
wv.loadUrl(thisUrl);
54+
}
55+
56+
//Test onCreate with both JavaScript and cross-origin resource access disabled by default while taking remote user inputs
57+
public void testOnCreate3(Bundle savedInstanceState) {
58+
super.onCreate(savedInstanceState);
59+
setContentView(-1);
60+
61+
WebView wv = (WebView) findViewById(-1);
62+
WebSettings webSettings = wv.getSettings();
63+
64+
wv.setWebViewClient(new WebViewClient() {
65+
@Override
66+
public boolean shouldOverrideUrlLoading(WebView view, String url) {
67+
view.loadUrl(url);
68+
return true;
69+
}
70+
});
71+
72+
String thisUrl = getIntent().getStringExtra("url");
73+
wv.loadUrl(thisUrl);
74+
}
75+
76+
//Test onCreate with JavaScript enabled but cross-origin resource access disabled while taking remote user inputs
77+
public void testOnCreate4(Bundle savedInstanceState) {
78+
super.onCreate(savedInstanceState);
79+
setContentView(-1);
80+
81+
WebView wv = (WebView) findViewById(-1);
82+
WebSettings webSettings = wv.getSettings();
83+
84+
webSettings.setJavaScriptEnabled(true);
85+
86+
wv.setWebViewClient(new WebViewClient() {
87+
@Override
88+
public boolean shouldOverrideUrlLoading(WebView view, String url) {
89+
view.loadUrl(url);
90+
return true;
91+
}
92+
});
93+
94+
String thisUrl = getIntent().getStringExtra("url");
95+
wv.loadUrl(thisUrl);
96+
}
97+
98+
//Test onCreate with both JavaScript and cross-origin resource access enabled while not taking remote user inputs
99+
public void testOnCreate5(Bundle savedInstanceState) {
100+
super.onCreate(savedInstanceState);
101+
setContentView(-1);
102+
103+
WebView wv = (WebView) findViewById(-1);
104+
WebSettings webSettings = wv.getSettings();
105+
106+
webSettings.setJavaScriptEnabled(true);
107+
webSettings.setAllowFileAccessFromFileURLs(true);
108+
109+
wv.setWebViewClient(new WebViewClient() {
110+
@Override
111+
public boolean shouldOverrideUrlLoading(WebView view, String url) {
112+
view.loadUrl(url);
113+
return true;
114+
}
115+
});
116+
117+
wv.loadUrl("https://www.mycorp.com");
118+
}
119+
}

‎java/ql/test/experimental/query-tests/security/CWE-749/UnsafeAndroidAccess.qlref‎

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

‎java/ql/test/experimental/query-tests/security/CWE-749/options‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
// semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/google-android-9.0.0

0 commit comments

Comments
 (0)