Skip to content

Commit 26999c7

Browse files
atorralbaatorralba
committed
Decouple UnsafeAndroidAccess.qll to reuse the taint tracking configuration
1 parent 99e66cf commit 26999c7

5 files changed

Lines changed: 34 additions & 53 deletions

File tree

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

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,26 +12,9 @@
1212
*/
1313

1414
import java
15-
import semmle.code.java.dataflow.FlowSources
16-
import semmle.code.java.security.RequestForgeryConfig
17-
import semmle.code.java.security.UnsafeAndroidAccess
15+
import semmle.code.java.security.UnsafeAndroidAccessQuery
1816
import DataFlow::PathGraph
1917

20-
/**
21-
* Taint configuration tracking flow from untrusted inputs to a resource fetching call.
22-
*/
23-
class FetchUntrustedResourceConfiguration extends TaintTracking::Configuration {
24-
FetchUntrustedResourceConfiguration() { this = "FetchUntrustedResourceConfiguration" }
25-
26-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
27-
28-
override predicate isSink(DataFlow::Node sink) { sink instanceof UrlResourceSink }
29-
30-
override predicate isSanitizer(DataFlow::Node sanitizer) {
31-
sanitizer instanceof RequestForgerySanitizer
32-
}
33-
}
34-
3518
from DataFlow::PathNode source, DataFlow::PathNode sink, FetchUntrustedResourceConfiguration conf
3619
where conf.hasFlowPath(source, sink)
3720
select sink.getNode(), source, sink, "Unsafe resource fetching in Android webview due to $@.",

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,6 @@ private module Frameworks {
9898
private import semmle.code.java.security.InformationLeak
9999
private import semmle.code.java.security.JexlInjectionSinkModels
100100
private import semmle.code.java.security.LdapInjection
101-
private import semmle.code.java.security.UnsafeAndroidAccess
102101
private import semmle.code.java.security.XPath
103102
private import semmle.code.java.frameworks.android.SQLite
104103
private import semmle.code.java.frameworks.Jdbc

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

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@
33
*/
44

55
import java
6-
import semmle.code.java.frameworks.android.WebView
7-
import semmle.code.java.dataflow.DataFlow
8-
import semmle.code.java.dataflow.ExternalFlow
6+
private import semmle.code.java.frameworks.android.WebView
7+
private import semmle.code.java.dataflow.DataFlow
8+
private import semmle.code.java.dataflow.ExternalFlow
99

1010
/**
1111
* A sink that represents a method that fetches a web resource in Android.
@@ -19,17 +19,6 @@ abstract class UrlResourceSink extends DataFlow::Node {
1919
abstract string getSinkType();
2020
}
2121

22-
/** CSV sink models representing methods susceptible to Unsafe Resource Fetching attacks. */
23-
private class DefaultUrlResourceSinkModel extends SinkModelCsv {
24-
override predicate row(string row) {
25-
row =
26-
[
27-
"android.webkit;WebView;true;loadUrl;;;Argument[0];unsafe-android-access",
28-
"android.webkit;WebView;true;postUrl;;;Argument[0];unsafe-android-access"
29-
]
30-
}
31-
}
32-
3322
/**
3423
* Cross-origin access enabled resource fetch.
3524
*
@@ -57,9 +46,10 @@ private class CrossOriginUrlResourceSink extends JavaScriptEnabledUrlResourceSin
5746
*/
5847
private class JavaScriptEnabledUrlResourceSink extends UrlResourceSink {
5948
JavaScriptEnabledUrlResourceSink() {
60-
sinkNode(this, "unsafe-android-access") and
61-
exists(VarAccess webviewVa, MethodAccess getSettingsMa, Variable v |
62-
this.asExpr().(Argument).getCall().getQualifier() = webviewVa and
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
6353
getSettingsMa.getMethod() instanceof WebViewGetSettingsMethod and
6454
webviewVa.getVariable().getAnAccess() = getSettingsMa.getQualifier() and
6555
v.getAnAssignedValue() = getSettingsMa and

‎java/ql/src/semmle/code/java/security/UnsafeAndroidAccessQuery.qll‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/** Provides taint tracking configurations to be used in Unsafe Resource Fetching queries. */
2+
3+
import java
4+
import semmle.code.java.dataflow.FlowSources
5+
import semmle.code.java.dataflow.TaintTracking
6+
import semmle.code.java.security.RequestForgery
7+
import semmle.code.java.security.UnsafeAndroidAccess
8+
9+
/**
10+
* Taint configuration tracking flow from untrusted inputs to a resource fetching call.
11+
*/
12+
class FetchUntrustedResourceConfiguration extends TaintTracking::Configuration {
13+
FetchUntrustedResourceConfiguration() { this = "FetchUntrustedResourceConfiguration" }
14+
15+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
16+
17+
override predicate isSink(DataFlow::Node sink) { sink instanceof UrlResourceSink }
18+
19+
override predicate isSanitizer(DataFlow::Node sanitizer) {
20+
sanitizer instanceof RequestForgerySanitizer
21+
}
22+
}

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

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,17 @@
11
import java
2-
import semmle.code.java.dataflow.DataFlow
3-
import semmle.code.java.dataflow.FlowSources
4-
import semmle.code.java.security.RequestForgeryConfig
5-
import semmle.code.java.security.UnsafeAndroidAccess
2+
import semmle.code.java.security.UnsafeAndroidAccessQuery
63
import TestUtilities.InlineExpectationsTest
74

8-
class Conf extends TaintTracking::Configuration {
9-
Conf() { this = "qltest:cwe:unsafe-android-access" }
10-
11-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
12-
13-
override predicate isSink(DataFlow::Node sink) { sink instanceof UrlResourceSink }
14-
15-
override predicate isSanitizer(DataFlow::Node sanitizer) {
16-
sanitizer instanceof RequestForgerySanitizer
17-
}
18-
}
19-
205
class UnsafeAndroidAccessTest extends InlineExpectationsTest {
216
UnsafeAndroidAccessTest() { this = "HasUnsafeAndroidAccess" }
227

238
override string getARelevantTag() { result = "hasUnsafeAndroidAccess" }
249

2510
override predicate hasActualResult(Location location, string element, string tag, string value) {
2611
tag = "hasUnsafeAndroidAccess" and
27-
exists(DataFlow::Node src, DataFlow::Node sink, Conf conf | conf.hasFlow(src, sink) |
12+
exists(DataFlow::Node src, DataFlow::Node sink, FetchUntrustedResourceConfiguration conf |
13+
conf.hasFlow(src, sink)
14+
|
2815
sink.getLocation() = location and
2916
element = sink.toString() and
3017
value = ""

0 commit comments

Comments
 (0)