Skip to content

Commit 3fcc9fa

Browse files
committed
Refactor sinks to reuse code
1 parent 6e3b6dc commit 3fcc9fa

File tree

1 file changed

+21
-13
lines changed

1 file changed

+21
-13
lines changed

java/ql/src/semmle/code/java/security/UnsafeAndroidAccess.qll

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,11 @@ abstract class UrlResourceSink extends DataFlow::Node {
2626
*/
2727
private class CrossOriginUrlResourceSink extends JavaScriptEnabledUrlResourceSink {
2828
CrossOriginUrlResourceSink() {
29-
exists(MethodAccess ma, MethodAccess getSettingsMa |
29+
exists(Variable settings, MethodAccess ma |
30+
webViewLoadUrl(this.asExpr(), settings) and
3031
ma.getMethod() instanceof CrossOriginAccessMethod and
3132
ma.getArgument(0).(BooleanLiteral).getBooleanValue() = true and
32-
ma.getQualifier().(VarAccess).getVariable().getAnAssignedValue() = getSettingsMa and
33-
getSettingsMa.getMethod() instanceof WebViewGetSettingsMethod and
34-
getSettingsMa.getQualifier().(VarAccess).getVariable().getAnAccess() =
35-
this.asExpr().(Argument).getCall().getQualifier()
33+
ma.getQualifier() = settings.getAnAccess()
3634
)
3735
}
3836

@@ -46,20 +44,30 @@ private class CrossOriginUrlResourceSink extends JavaScriptEnabledUrlResourceSin
4644
*/
4745
private class JavaScriptEnabledUrlResourceSink extends UrlResourceSink {
4846
JavaScriptEnabledUrlResourceSink() {
49-
exists(MethodAccess loadUrl, VarAccess webviewVa, MethodAccess getSettingsMa, Variable v |
50-
loadUrl.getArgument(0) = this.asExpr() and
51-
loadUrl.getMethod() instanceof WebViewLoadUrlMethod and
52-
loadUrl.getQualifier() = webviewVa and
53-
getSettingsMa.getMethod() instanceof WebViewGetSettingsMethod and
54-
webviewVa.getVariable().getAnAccess() = getSettingsMa.getQualifier() and
55-
v.getAnAssignedValue() = getSettingsMa and
56-
isJSEnabled(v)
47+
exists(Variable settings |
48+
webViewLoadUrl(this.asExpr(), settings) and
49+
isJSEnabled(settings)
5750
)
5851
}
5952

6053
override string getSinkType() { result = "user input vulnerable to XSS attacks" }
6154
}
6255

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+
}
70+
6371
/**
6472
* A method allowing any-local-file and cross-origin access in the WebSettings class.
6573
*/

0 commit comments

Comments
 (0)