Skip to content

Commit 91bdb42

Browse files
atorralbaatorralba
committed
Improvements to UnsafeAndroidAccess
1 parent 9c92454 commit 91bdb42

File tree

3 files changed

+166
-43
lines changed

3 files changed

+166
-43
lines changed

‎java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ private module Frameworks {
8686
private import semmle.code.java.frameworks.android.Slice
8787
private import semmle.code.java.frameworks.android.SQLite
8888
private import semmle.code.java.frameworks.android.Widget
89+
private import semmle.code.java.frameworks.android.WebView
8990
private import semmle.code.java.frameworks.android.XssSinks
9091
private import semmle.code.java.frameworks.ApacheHttp
9192
private import semmle.code.java.frameworks.apache.Collections

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

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,140 @@
11
import java
2+
private import semmle.code.java.dataflow.DataFlow
3+
private import semmle.code.java.dataflow.ExternalFlow
24

5+
/** The class `android.webkit.WebView`. */
36
class TypeWebView extends Class {
47
TypeWebView() { this.hasQualifiedName("android.webkit", "WebView") }
58
}
69

10+
/** The class `android.webkit.WebViewClient`. */
711
class TypeWebViewClient extends Class {
812
TypeWebViewClient() { this.hasQualifiedName("android.webkit", "WebViewClient") }
913
}
1014

15+
/** The class `android.webkit.WebSettings`. */
1116
class TypeWebSettings extends Class {
1217
TypeWebSettings() { this.hasQualifiedName("android.webkit", "WebSettings") }
1318
}
1419

20+
/** The method `getSettings` of the class `android.webkit.WebView`. */
1521
class WebViewGetSettingsMethod extends Method {
1622
WebViewGetSettingsMethod() {
1723
this.hasName("getSettings") and
1824
this.getDeclaringType() instanceof TypeWebView
1925
}
2026
}
2127

28+
/** The method `loadUrl` or `postUrl` of the class `android.webkit.WebView`. */
2229
class WebViewLoadUrlMethod extends Method {
2330
WebViewLoadUrlMethod() {
2431
this.getDeclaringType() instanceof TypeWebView and
2532
(this.hasName("loadUrl") or this.hasName("postUrl"))
2633
}
2734
}
2835

36+
/** The method `getUrl` or `getOriginalUrl` of the class `android.webkit.WebView`. */
2937
class WebViewGetUrlMethod extends Method {
3038
WebViewGetUrlMethod() {
3139
this.getDeclaringType() instanceof TypeWebView and
3240
(this.getName() = "getUrl" or this.getName() = "getOriginalUrl")
3341
}
3442
}
43+
44+
/**
45+
* A method allowing any-local-file and cross-origin access in the class `android.webkit.WebSettings`.
46+
*/
47+
class CrossOriginAccessMethod extends Method {
48+
CrossOriginAccessMethod() {
49+
this.getDeclaringType() instanceof TypeWebSettings and
50+
this.hasName(["setAllowUniversalAccessFromFileURLs", "setAllowFileAccessFromFileURLs"])
51+
}
52+
}
53+
54+
/**
55+
* The method `setJavaScriptEnabled` of the class `android.webkit.WebSettings`.
56+
*/
57+
class AllowJavaScriptMethod extends Method {
58+
AllowJavaScriptMethod() {
59+
this.getDeclaringType() instanceof TypeWebSettings and
60+
this.hasName("setJavaScriptEnabled")
61+
}
62+
}
63+
64+
/** The method `setWebViewClient` of the class `android.webkit.WebView`. */
65+
class WebViewSetWebViewClientMethod extends Method {
66+
WebViewSetWebViewClientMethod() {
67+
this.getDeclaringType() instanceof TypeWebView and
68+
this.hasName("setWebViewClient")
69+
}
70+
}
71+
72+
/** The method `shouldOverrideUrlLoading` of the class `android.webkit.WebViewClient`. */
73+
class ShouldOverrideUrlLoading extends Method {
74+
ShouldOverrideUrlLoading() {
75+
this.getDeclaringType().getASupertype*() instanceof TypeWebViewClient and
76+
this.hasName("shouldOverrideUrlLoading")
77+
}
78+
}
79+
80+
/**
81+
* Holds if `webview` is a `WebView` and its option `setJavascriptEnabled`
82+
* has been set to `true` via a `WebSettings` object obtained from it.
83+
*/
84+
predicate isJSEnabled(Expr webview) {
85+
webview.getType().(RefType).getASupertype*() instanceof TypeWebView and
86+
exists(MethodAccess allowJs |
87+
allowJs.getMethod() instanceof AllowJavaScriptMethod and
88+
allowJs.getArgument(0).(CompileTimeConstantExpr).getBooleanValue() = true and
89+
exists(MethodAccess settings |
90+
settings.getMethod() instanceof WebViewGetSettingsMethod and
91+
DataFlow::localExprFlow(settings, allowJs.getQualifier()) and
92+
DataFlow::localExprFlow(webview, settings.getQualifier())
93+
)
94+
)
95+
}
96+
97+
/**
98+
* Holds if `webview` is a `WebView` and its options `setAllowUniversalAccessFromFileURLs` or
99+
* `setAllowFileAccessFromFileURLs` have been set to `true`.
100+
*/
101+
predicate isAllowFileAccessEnabled(Expr webview) {
102+
exists(MethodAccess allowFileAccess |
103+
allowFileAccess.getMethod() instanceof CrossOriginAccessMethod and
104+
allowFileAccess.getArgument(0).(CompileTimeConstantExpr).getBooleanValue() = true and
105+
exists(MethodAccess settings |
106+
settings.getMethod() instanceof WebViewGetSettingsMethod and
107+
DataFlow::localExprFlow(settings, allowFileAccess.getQualifier()) and
108+
DataFlow::localExprFlow(webview, settings.getQualifier())
109+
)
110+
)
111+
}
112+
113+
private class WebkitSourceModels extends SourceModelCsv {
114+
override predicate row(string row) {
115+
row =
116+
[
117+
"android.webkit;WebResourceRequest;true;doUpdateVisitedHistory;;;Parameter[1];remote",
118+
"android.webkit;WebResourceRequest;true;onLoadResource;;;Parameter[1];remote",
119+
"android.webkit;WebResourceRequest;true;onPageCommitVisible;;;Parameter[1];remote",
120+
"android.webkit;WebResourceRequest;true;onPageFinished;;;Parameter[1];remote",
121+
"android.webkit;WebResourceRequest;true;onPageStarted;;;Parameter[1];remote",
122+
"android.webkit;WebResourceRequest;true;onReceivedError;(WebView,int,String,String);;Parameter[3];remote",
123+
"android.webkit;WebResourceRequest;true;onReceivedError;(WebView,WebResourceRequest,WebResourceError);;Parameter[1];remote",
124+
"android.webkit;WebResourceRequest;true;onReceivedHttpError;;;Parameter[1];remote",
125+
"android.webkit;WebResourceRequest;true;onSafeBrowsingHit;;;Parameter[1];remote",
126+
"android.webkit;WebResourceRequest;true;shouldInterceptRequest;;;Parameter[1];remote",
127+
"android.webkit;WebResourceRequest;true;shouldOverrideUrlLoading;;;Parameter[1];remote"
128+
]
129+
}
130+
}
131+
132+
private class WebkitSummaryModels extends SummaryModelCsv {
133+
override predicate row(string row) {
134+
row =
135+
[
136+
"android.webkit;WebResourceRequest;true;getRequestHeaders;;;Argument[-1];ReturnValue;taint",
137+
"android.webkit;WebResourceRequest;true;getUrl;;;Argument[-1];ReturnValue;taint"
138+
]
139+
}
140+
}

‎java/ql/lib/semmle/code/java/security/UnsafeAndroidAccess.qll

Lines changed: 59 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,9 @@ abstract class UrlResourceSink extends DataFlow::Node {
2626
*/
2727
private class CrossOriginUrlResourceSink extends JavaScriptEnabledUrlResourceSink {
2828
CrossOriginUrlResourceSink() {
29-
exists(Variable settings, MethodAccess ma |
30-
webViewLoadUrl(this.asExpr(), settings) and
31-
ma.getMethod() instanceof CrossOriginAccessMethod and
32-
ma.getArgument(0).(BooleanLiteral).getBooleanValue() = true and
33-
ma.getQualifier() = settings.getAnAccess()
29+
exists(WebViewRef webview |
30+
webViewLoadUrl(this.asExpr(), webview.getAnAccess()) and
31+
isAllowFileAccessEnabled(webview.getAnAccess())
3432
)
3533
}
3634

@@ -44,57 +42,75 @@ private class CrossOriginUrlResourceSink extends JavaScriptEnabledUrlResourceSin
4442
*/
4543
private class JavaScriptEnabledUrlResourceSink extends UrlResourceSink {
4644
JavaScriptEnabledUrlResourceSink() {
47-
exists(Variable settings |
48-
webViewLoadUrl(this.asExpr(), settings) and
49-
isJSEnabled(settings)
45+
exists(WebViewRef webview |
46+
isJSEnabled(webview.getAnAccess()) and
47+
webViewLoadUrl(this.asExpr(), webview.getAnAccess())
5048
)
5149
}
5250

5351
override string getSinkType() { result = "user input vulnerable to XSS attacks" }
5452
}
5553

56-
/**
57-
* Holds if a `WebViewLoadUrlMethod` method is called with the given `urlArg` on a
58-
* WebView with settings stored in `settings`.
59-
*/
60-
private predicate webViewLoadUrl(Expr urlArg, Variable settings) {
61-
exists(MethodAccess loadUrl, Variable webview, MethodAccess getSettings |
62-
loadUrl.getArgument(0) = urlArg and
63-
loadUrl.getMethod() instanceof WebViewLoadUrlMethod and
64-
loadUrl.getQualifier() = webview.getAnAccess() and
65-
getSettings.getMethod() instanceof WebViewGetSettingsMethod and
66-
webview.getAnAccess() = getSettings.getQualifier() and
67-
settings.getAnAssignedValue() = getSettings
68-
)
69-
}
54+
private class WebViewRef extends Element {
55+
WebViewRef() {
56+
this.(RefType).getASourceSupertype*() instanceof TypeWebView or
57+
this.(Variable).getType().(RefType).getASourceSupertype*() instanceof TypeWebView
58+
}
7059

71-
/**
72-
* A method allowing any-local-file and cross-origin access in the WebSettings class.
73-
*/
74-
private class CrossOriginAccessMethod extends Method {
75-
CrossOriginAccessMethod() {
76-
this.getDeclaringType() instanceof TypeWebSettings and
77-
this.hasName(["setAllowUniversalAccessFromFileURLs", "setAllowFileAccessFromFileURLs"])
60+
/** Gets an access to this WebView. */
61+
Expr getAnAccess() {
62+
exists(ThisAccess t | t.getType() = this and result = t |
63+
t.isOwnInstanceAccess() or
64+
t.isEnclosingInstanceAccess(this)
65+
)
66+
or
67+
result = this.(Variable).getAnAccess()
7868
}
7969
}
8070

81-
/**
82-
* The `setJavaScriptEnabled` method for the webview.
83-
*/
84-
private class AllowJavaScriptMethod extends Method {
85-
AllowJavaScriptMethod() {
86-
this.getDeclaringType() instanceof TypeWebSettings and
87-
this.hasName("setJavaScriptEnabled")
88-
}
71+
private Expr getUnderlyingExpr(Expr e) {
72+
if e instanceof CastExpr or e instanceof UnaryExpr
73+
then
74+
result = getUnderlyingExpr(e.(CastExpr).getExpr()) or
75+
result = getUnderlyingExpr(e.(UnaryExpr).getExpr())
76+
else result = e
8977
}
9078

9179
/**
92-
* Holds if a call to `v.setJavaScriptEnabled(true)` exists.
80+
* Holds if `WebViewLoadUrlMethod` is called on `webview`
81+
* with `urlArg` as its first argument.
9382
*/
94-
private predicate isJSEnabled(Variable v) {
95-
exists(MethodAccess jsa |
96-
v.getAnAccess() = jsa.getQualifier() and
97-
jsa.getMethod() instanceof AllowJavaScriptMethod and
98-
jsa.getArgument(0).(BooleanLiteral).getBooleanValue() = true
83+
private predicate webViewLoadUrl(Argument urlArg, Expr webview) {
84+
exists(MethodAccess loadUrl |
85+
loadUrl.getArgument(0) = urlArg and
86+
loadUrl.getMethod() instanceof WebViewLoadUrlMethod
87+
|
88+
getUnderlyingExpr(loadUrl.getQualifier()) = webview
89+
or
90+
// `webview` is received as a parameter of an event method in a custom `WebViewClient`,
91+
// so we need to find WebViews that use that specific `WebViewClient`.
92+
exists(WebViewClientEventMethod eventMethod, MethodAccess setWebClient |
93+
setWebClient.getMethod() instanceof WebViewSetWebViewClientMethod and
94+
setWebClient.getArgument(0).getType() = eventMethod.getDeclaringType() and
95+
getUnderlyingExpr(setWebClient.getQualifier()) = webview and
96+
getUnderlyingExpr(loadUrl.getQualifier()) = eventMethod.getWebViewParameter().getAnAccess()
97+
)
9998
)
10099
}
100+
101+
/** A method of the class `WebViewClient` that handles an event. */
102+
private class WebViewClientEventMethod extends Method {
103+
WebViewClientEventMethod() {
104+
this.getDeclaringType().getASupertype*() instanceof TypeWebViewClient and
105+
this.hasName([
106+
"shouldOverrideUrlLoading", "shouldInterceptRequest", "onPageStarted", "onPageFinished",
107+
"onLoadResource", "onPageCommitVisible", "onTooManyRedirects"
108+
])
109+
}
110+
111+
/** Gets a `WebView` parameter of this method. */
112+
Parameter getWebViewParameter() {
113+
result = this.getAParameter() and
114+
result.getType() instanceof TypeWebView
115+
}
116+
}

0 commit comments

Comments
 (0)