Skip to content

Commit 651b73e

Browse files
committed
Swift: Check for tainted baseURL.
1 parent 53ea65b commit 651b73e

File tree

3 files changed

+48
-5
lines changed

3 files changed

+48
-5
lines changed

swift/ql/src/queries/Security/CWE-095/UnsafeWebViewFetch.ql

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,8 @@ class UnsafeWebViewFetchConfig extends TaintTracking::Configuration {
9696
override predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
9797

9898
override predicate isSink(DataFlow::Node node) {
99-
node instanceof Sink
99+
node instanceof Sink or
100+
node.asExpr() = any(Sink s).getBaseURL()
100101
}
101102

102103
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
@@ -111,6 +112,16 @@ class UnsafeWebViewFetchConfig extends TaintTracking::Configuration {
111112
// allow flow through string concatenation.
112113
// TODO: this should probably be part of TaintTracking.
113114
node2.asExpr().(AddExpr).getAnOperand() = node1.asExpr()
115+
or
116+
// allow flow through `URL.init`.
117+
exists(CallExpr call, ClassDecl c, AbstractFunctionDecl f |
118+
c.getName() = "URL" and
119+
c.getAMember() = f and
120+
f.getName() = ["init(string:)", "init(string:relativeTo:)"] and
121+
call.getFunction().(ApplyExpr).getStaticTarget() = f and
122+
node1.asExpr() = call.getArgument(_).getExpr() and
123+
node2.asExpr() = call
124+
)
114125
}
115126
}
116127

@@ -124,5 +135,9 @@ where
124135
// base URL is nil
125136
sink.getBaseURL() instanceof NilLiteralExpr and
126137
message = "Tainted data is used in a WebView fetch without restricting the base URL."
138+
or
139+
// base URL is tainted
140+
config.hasFlow(_, any(DataFlow::Node n | n.asExpr() = sink.getBaseURL())) and
141+
message = "Tainted data is used in a WebView fetch with a tainted base URL."
127142
)
128143
select sinkNode, sourceNode, sinkNode, message

swift/ql/test/query-tests/Security/CWE-095/UnsafeWebViewFetch.expected

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,26 @@ edges
88
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:124:25:124:51 | ... call to +(_:_:) ... |
99
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:135:25:135:25 | remoteString |
1010
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:137:25:137:25 | remoteString |
11+
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:138:47:138:56 | ...! |
1112
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:139:25:139:25 | remoteString |
13+
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:139:48:139:57 | ...! |
14+
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:140:47:140:57 | ...! |
1215
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:141:25:141:25 | remoteString |
16+
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:141:48:141:58 | ...! |
17+
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:153:85:153:94 | ...! |
18+
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:154:86:154:95 | ...! |
1319
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:168:25:168:25 | remoteString |
1420
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:171:25:171:51 | ... call to +(_:_:) ... |
1521
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:182:25:182:25 | remoteString |
1622
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:184:25:184:25 | remoteString |
23+
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:185:47:185:56 | ...! |
1724
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:186:25:186:25 | remoteString |
25+
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:186:48:186:57 | ...! |
26+
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:187:47:187:57 | ...! |
1827
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:188:25:188:25 | remoteString |
28+
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:188:48:188:58 | ...! |
29+
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:200:90:200:99 | ...! |
30+
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:201:91:201:100 | ...! |
1931
nodes
2032
| UnsafeWebViewFetch.swift:94:10:94:37 | try ... : | semmle.label | try ... : |
2133
| UnsafeWebViewFetch.swift:94:14:94:37 | call to ... : | semmle.label | call to ... : |
@@ -25,21 +37,37 @@ nodes
2537
| UnsafeWebViewFetch.swift:124:25:124:51 | ... call to +(_:_:) ... | semmle.label | ... call to +(_:_:) ... |
2638
| UnsafeWebViewFetch.swift:135:25:135:25 | remoteString | semmle.label | remoteString |
2739
| UnsafeWebViewFetch.swift:137:25:137:25 | remoteString | semmle.label | remoteString |
40+
| UnsafeWebViewFetch.swift:138:47:138:56 | ...! | semmle.label | ...! |
2841
| UnsafeWebViewFetch.swift:139:25:139:25 | remoteString | semmle.label | remoteString |
42+
| UnsafeWebViewFetch.swift:139:48:139:57 | ...! | semmle.label | ...! |
43+
| UnsafeWebViewFetch.swift:140:47:140:57 | ...! | semmle.label | ...! |
2944
| UnsafeWebViewFetch.swift:141:25:141:25 | remoteString | semmle.label | remoteString |
45+
| UnsafeWebViewFetch.swift:141:48:141:58 | ...! | semmle.label | ...! |
46+
| UnsafeWebViewFetch.swift:153:85:153:94 | ...! | semmle.label | ...! |
47+
| UnsafeWebViewFetch.swift:154:86:154:95 | ...! | semmle.label | ...! |
3048
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | semmle.label | call to getRemoteData() : |
3149
| UnsafeWebViewFetch.swift:167:25:167:39 | call to getRemoteData() | semmle.label | call to getRemoteData() |
3250
| UnsafeWebViewFetch.swift:168:25:168:25 | remoteString | semmle.label | remoteString |
3351
| UnsafeWebViewFetch.swift:171:25:171:51 | ... call to +(_:_:) ... | semmle.label | ... call to +(_:_:) ... |
3452
| UnsafeWebViewFetch.swift:182:25:182:25 | remoteString | semmle.label | remoteString |
3553
| UnsafeWebViewFetch.swift:184:25:184:25 | remoteString | semmle.label | remoteString |
54+
| UnsafeWebViewFetch.swift:185:47:185:56 | ...! | semmle.label | ...! |
3655
| UnsafeWebViewFetch.swift:186:25:186:25 | remoteString | semmle.label | remoteString |
56+
| UnsafeWebViewFetch.swift:186:48:186:57 | ...! | semmle.label | ...! |
57+
| UnsafeWebViewFetch.swift:187:47:187:57 | ...! | semmle.label | ...! |
3758
| UnsafeWebViewFetch.swift:188:25:188:25 | remoteString | semmle.label | remoteString |
59+
| UnsafeWebViewFetch.swift:188:48:188:58 | ...! | semmle.label | ...! |
60+
| UnsafeWebViewFetch.swift:200:90:200:99 | ...! | semmle.label | ...! |
61+
| UnsafeWebViewFetch.swift:201:91:201:100 | ...! | semmle.label | ...! |
3862
subpaths
3963
#select
4064
| UnsafeWebViewFetch.swift:120:25:120:39 | call to getRemoteData() | UnsafeWebViewFetch.swift:94:14:94:37 | call to ... : | UnsafeWebViewFetch.swift:120:25:120:39 | call to getRemoteData() | Tainted data is used in a WebView fetch without restricting the base URL. |
4165
| UnsafeWebViewFetch.swift:121:25:121:25 | remoteString | UnsafeWebViewFetch.swift:94:14:94:37 | call to ... : | UnsafeWebViewFetch.swift:121:25:121:25 | remoteString | Tainted data is used in a WebView fetch without restricting the base URL. |
4266
| UnsafeWebViewFetch.swift:124:25:124:51 | ... call to +(_:_:) ... | UnsafeWebViewFetch.swift:94:14:94:37 | call to ... : | UnsafeWebViewFetch.swift:124:25:124:51 | ... call to +(_:_:) ... | Tainted data is used in a WebView fetch without restricting the base URL. |
67+
| UnsafeWebViewFetch.swift:139:25:139:25 | remoteString | UnsafeWebViewFetch.swift:94:14:94:37 | call to ... : | UnsafeWebViewFetch.swift:139:25:139:25 | remoteString | Tainted data is used in a WebView fetch with a tainted base URL. |
68+
| UnsafeWebViewFetch.swift:141:25:141:25 | remoteString | UnsafeWebViewFetch.swift:94:14:94:37 | call to ... : | UnsafeWebViewFetch.swift:141:25:141:25 | remoteString | Tainted data is used in a WebView fetch with a tainted base URL. |
4369
| UnsafeWebViewFetch.swift:167:25:167:39 | call to getRemoteData() | UnsafeWebViewFetch.swift:94:14:94:37 | call to ... : | UnsafeWebViewFetch.swift:167:25:167:39 | call to getRemoteData() | Tainted data is used in a WebView fetch without restricting the base URL. |
4470
| UnsafeWebViewFetch.swift:168:25:168:25 | remoteString | UnsafeWebViewFetch.swift:94:14:94:37 | call to ... : | UnsafeWebViewFetch.swift:168:25:168:25 | remoteString | Tainted data is used in a WebView fetch without restricting the base URL. |
4571
| UnsafeWebViewFetch.swift:171:25:171:51 | ... call to +(_:_:) ... | UnsafeWebViewFetch.swift:94:14:94:37 | call to ... : | UnsafeWebViewFetch.swift:171:25:171:51 | ... call to +(_:_:) ... | Tainted data is used in a WebView fetch without restricting the base URL. |
72+
| UnsafeWebViewFetch.swift:186:25:186:25 | remoteString | UnsafeWebViewFetch.swift:94:14:94:37 | call to ... : | UnsafeWebViewFetch.swift:186:25:186:25 | remoteString | Tainted data is used in a WebView fetch with a tainted base URL. |
73+
| UnsafeWebViewFetch.swift:188:25:188:25 | remoteString | UnsafeWebViewFetch.swift:94:14:94:37 | call to ... : | UnsafeWebViewFetch.swift:188:25:188:25 | remoteString | Tainted data is used in a WebView fetch with a tainted base URL. |

swift/ql/test/query-tests/Security/CWE-095/UnsafeWebViewFetch.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,9 +136,9 @@ func testUIWebView() {
136136
webview.loadHTMLString(localString, baseURL: localURL!) // GOOD: a presumed safe baseURL is specified
137137
webview.loadHTMLString(remoteString, baseURL: localURL!) // GOOD: a presumed safe baseURL is specified
138138
webview.loadHTMLString(localString, baseURL: remoteURL!) // GOOD: the HTML data is local
139-
webview.loadHTMLString(remoteString, baseURL: remoteURL!) // BAD [NOT DETECTED]
139+
webview.loadHTMLString(remoteString, baseURL: remoteURL!) // BAD
140140
webview.loadHTMLString(localString, baseURL: remoteURL2!) // GOOD: the HTML data is local
141-
webview.loadHTMLString(remoteString, baseURL: remoteURL2!) // BAD [NOT DETECTED]
141+
webview.loadHTMLString(remoteString, baseURL: remoteURL2!) // BAD
142142

143143
let localRequest = URLRequest(url: localURL!)
144144
let remoteRequest = URLRequest(url: remoteURL!)
@@ -183,9 +183,9 @@ func testWKWebView() {
183183
webview.loadHTMLString(localString, baseURL: localURL!) // GOOD: a presumed safe baseURL is specified
184184
webview.loadHTMLString(remoteString, baseURL: localURL!) // GOOD: a presumed safe baseURL is specified
185185
webview.loadHTMLString(localString, baseURL: remoteURL!) // GOOD: the HTML data is local
186-
webview.loadHTMLString(remoteString, baseURL: remoteURL!) // BAD [NOT DETECTED]
186+
webview.loadHTMLString(remoteString, baseURL: remoteURL!) // BAD
187187
webview.loadHTMLString(localString, baseURL: remoteURL2!) // GOOD: the HTML data is local
188-
webview.loadHTMLString(remoteString, baseURL: remoteURL2!) // BAD [NOT DETECTED]
188+
webview.loadHTMLString(remoteString, baseURL: remoteURL2!) // BAD
189189

190190
let localRequest = URLRequest(url: localURL!)
191191
let remoteRequest = URLRequest(url: remoteURL!)

0 commit comments

Comments
 (0)