Skip to content

Commit 1c8090f

Browse files
MathiasVPMathiasVP
authored
Merge pull request #9964 from geoffw0/cwe95
Swift: Query for CWE-79 / CWE-95
2 parents ac26371 + 1ce06ac commit 1c8090f

File tree

8 files changed

+488
-0
lines changed

8 files changed

+488
-0
lines changed

‎swift/ql/lib/codeql/swift/dataflow/FlowSources.qll

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ abstract class RemoteFlowSource extends Node {
1111
abstract string getSourceType();
1212
}
1313

14+
/**
15+
* A data flow source of remote user input that is defined through 'models as data'.
16+
*/
1417
private class ExternalRemoteFlowSource extends RemoteFlowSource {
1518
ExternalRemoteFlowSource() { sourceNode(this, "remote") }
1619

‎swift/ql/src/queries/Security/CWE-079/UnsafeWebViewFetch.qhelp

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>Fetching data in a WebView without restricting the base URL may allow an attacker to access sensitive local data, for example using <code>file://</code>. Data can then be extracted from the software using the URL of a machine under the attackers control. More generally, an attacker may use a URL under their control as part of a cross-site scripting attack.</p>
7+
8+
</overview>
9+
<recommendation>
10+
11+
<p>When loading HTML into a web view, always set the <code>baseURL</code> to an appropriate URL that you control, or to <code>about:blank</code>. Do not use <code>nil</code>, as this does not restrict URLs that can be resolved. Also do not use a <code>baseURL</code> that could itself be controlled by an attacker.</p>
12+
13+
</recommendation>
14+
<example>
15+
16+
<p>In the following example, a call to <code>UIWebView.loadHTMLString</code> has the <code>baseURL</code> set to <code>nil</code>, which does not restrict URLs that can be resolved from within the web page.</p>
17+
18+
<sample src="UnsafeWebViewFetchBad.swift" />
19+
20+
<p>To fix the problem, we set the <code>baseURL</code> to <code>about:blank</code>. This ensures that an attacker cannot resolve URLs that point to the local file system, or to web servers under their control.</p>
21+
22+
<sample src="UnsafeWebViewFetchGood.swift" />
23+
24+
</example>
25+
<references>
26+
27+
<li>
28+
<a href="https://www.allysonomalley.com/2018/12/03/ios-bug-hunting-web-view-xss/">iOS Bug Hunting - Web View XSS</a>
29+
</li>
30+
31+
</references>
32+
</qhelp>

‎swift/ql/src/queries/Security/CWE-079/UnsafeWebViewFetch.ql

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
/**
2+
* @name Unsafe WebView fetch
3+
* @description Fetching data in a WebView without restricting the base URL may allow an attacker to access sensitive local data, or enable cross-site scripting attack.
4+
* @kind path-problem
5+
* @problem.severity warning
6+
* @security-severity 6.1
7+
* @precision high
8+
* @id swift/unsafe-webview-fetch
9+
* @tags security
10+
* external/cwe/cwe-079
11+
* external/cwe/cwe-095
12+
* external/cwe/cwe-749
13+
*/
14+
15+
import swift
16+
import codeql.swift.dataflow.DataFlow
17+
import codeql.swift.dataflow.TaintTracking
18+
import codeql.swift.dataflow.FlowSources
19+
import DataFlow::PathGraph
20+
import codeql.swift.frameworks.StandardLibrary.String
21+
22+
/**
23+
* A taint source that is `String(contentsOf:)`.
24+
* TODO: this shouldn't be needed when `StringSource` in `String.qll` is working.
25+
*/
26+
class StringContentsOfUrlSource extends RemoteFlowSource {
27+
StringContentsOfUrlSource() {
28+
exists(CallExpr call, AbstractFunctionDecl f |
29+
call.getFunction().(ApplyExpr).getStaticTarget() = f and
30+
f.getName() = "init(contentsOf:)" and
31+
f.getParam(0).getType().getName() = "URL" and
32+
this.asExpr() = call
33+
)
34+
}
35+
36+
override string getSourceType() { result = "" }
37+
}
38+
39+
/**
40+
* A sink that is a candidate result for this query, such as certain arguments
41+
* to `UIWebView.loadHTMLString`.
42+
*/
43+
class Sink extends DataFlow::Node {
44+
Expr baseUrl;
45+
46+
Sink() {
47+
exists(
48+
AbstractFunctionDecl funcDecl, CallExpr call, string funcName, string paramName, int arg,
49+
int baseUrlArg
50+
|
51+
// arguments to method calls...
52+
exists(string className, ClassDecl c |
53+
(
54+
// `loadHTMLString`
55+
className = ["UIWebView", "WKWebView"] and
56+
funcName = "loadHTMLString(_:baseURL:)" and
57+
paramName = "string"
58+
or
59+
// `UIWebView.load`
60+
className = "UIWebView" and
61+
funcName = "load(_:mimeType:textEncodingName:baseURL:)" and
62+
paramName = "data"
63+
or
64+
// `WKWebView.load`
65+
className = "WKWebView" and
66+
funcName = "load(_:mimeType:characterEncodingName:baseURL:)" and
67+
paramName = "data"
68+
) and
69+
c.getName() = className and
70+
c.getAMember() = funcDecl and
71+
call.getFunction().(ApplyExpr).getStaticTarget() = funcDecl
72+
) and
73+
// match up `funcName`, `paramName`, `arg`, `node`.
74+
funcDecl.getName() = funcName and
75+
funcDecl.getParam(pragma[only_bind_into](arg)).getName() = paramName and
76+
call.getArgument(pragma[only_bind_into](arg)).getExpr() = this.asExpr() and
77+
// match up `baseURLArg`
78+
funcDecl.getParam(pragma[only_bind_into](baseUrlArg)).getName() = "baseURL" and
79+
call.getArgument(pragma[only_bind_into](baseUrlArg)).getExpr() = baseUrl
80+
)
81+
}
82+
83+
/**
84+
* Gets the `baseURL` argument associated with this sink.
85+
*/
86+
Expr getBaseUrl() { result = baseUrl }
87+
}
88+
89+
/**
90+
* A taint configuration from taint sources to sinks (and `baseURL` arguments)
91+
* for this query.
92+
*/
93+
class UnsafeWebViewFetchConfig extends TaintTracking::Configuration {
94+
UnsafeWebViewFetchConfig() { this = "UnsafeWebViewFetchConfig" }
95+
96+
override predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
97+
98+
override predicate isSink(DataFlow::Node node) {
99+
node instanceof Sink or
100+
node.asExpr() = any(Sink s).getBaseUrl()
101+
}
102+
103+
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
104+
// allow flow through `try!` and similar constructs
105+
// TODO: this should probably be part of DataFlow / TaintTracking.
106+
node1.asExpr() = node2.asExpr().(AnyTryExpr).getSubExpr()
107+
or
108+
// allow flow through `!`
109+
// TODO: this should probably be part of DataFlow / TaintTracking.
110+
node1.asExpr() = node2.asExpr().(ForceValueExpr).getSubExpr()
111+
or
112+
// allow flow through string concatenation.
113+
// TODO: this should probably be part of TaintTracking.
114+
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.getAnArgument().getExpr() and
123+
node2.asExpr() = call
124+
)
125+
}
126+
}
127+
128+
from
129+
UnsafeWebViewFetchConfig config, DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode,
130+
Sink sink, string message
131+
where
132+
config.hasFlowPath(sourceNode, sinkNode) and
133+
sink = sinkNode.getNode() and
134+
(
135+
// base URL is nil
136+
sink.getBaseUrl() instanceof NilLiteralExpr and
137+
message = "Tainted data is used in a WebView fetch without restricting the base URL."
138+
or
139+
// base URL is tainted
140+
config.hasFlowToExpr(sink.getBaseUrl()) and
141+
message = "Tainted data is used in a WebView fetch with a tainted base URL."
142+
)
143+
select sink, sourceNode, sinkNode, message

‎swift/ql/src/queries/Security/CWE-079/UnsafeWebViewFetchBad.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
2+
let webview = UIWebView()
3+
4+
...
5+
6+
webview.loadHTMLString(htmlData, baseURL: nil) // BAD

‎swift/ql/src/queries/Security/CWE-079/UnsafeWebViewFetchGood.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
2+
let webview = UIWebView()
3+
4+
...
5+
6+
webview.loadHTMLString(htmlData, baseURL: URL(string: "about:blank")) // GOOD

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

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
edges
2+
| UnsafeWebViewFetch.swift:94:10:94:37 | try ... : | UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : |
3+
| UnsafeWebViewFetch.swift:94:10:94:37 | try ... : | UnsafeWebViewFetch.swift:120:25:120:39 | call to getRemoteData() |
4+
| UnsafeWebViewFetch.swift:94:10:94:37 | try ... : | UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : |
5+
| UnsafeWebViewFetch.swift:94:10:94:37 | try ... : | UnsafeWebViewFetch.swift:167:25:167:39 | call to getRemoteData() |
6+
| UnsafeWebViewFetch.swift:94:10:94:37 | try ... : | UnsafeWebViewFetch.swift:206:17:206:31 | call to getRemoteData() : |
7+
| UnsafeWebViewFetch.swift:94:14:94:37 | call to ... : | UnsafeWebViewFetch.swift:94:10:94:37 | try ... : |
8+
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:121:25:121:25 | remoteString |
9+
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:124:25:124:51 | ... call to +(_:_:) ... |
10+
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:135:25:135:25 | remoteString |
11+
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:137:25:137:25 | remoteString |
12+
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:138:47:138:56 | ...! |
13+
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:139:25:139:25 | remoteString |
14+
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:139:48:139:57 | ...! |
15+
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:140:47:140:57 | ...! |
16+
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:141:25:141:25 | remoteString |
17+
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:141:48:141:58 | ...! |
18+
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:153:85:153:94 | ...! |
19+
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:154:86:154:95 | ...! |
20+
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:168:25:168:25 | remoteString |
21+
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:171:25:171:51 | ... call to +(_:_:) ... |
22+
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:182:25:182:25 | remoteString |
23+
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:184:25:184:25 | remoteString |
24+
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:185:47:185:56 | ...! |
25+
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:186:25:186:25 | remoteString |
26+
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:186:48:186:57 | ...! |
27+
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:187:47:187:57 | ...! |
28+
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:188:25:188:25 | remoteString |
29+
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:188:48:188:58 | ...! |
30+
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:200:90:200:99 | ...! |
31+
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:201:91:201:100 | ...! |
32+
| UnsafeWebViewFetch.swift:206:17:206:31 | call to getRemoteData() : | UnsafeWebViewFetch.swift:210:25:210:25 | htmlData |
33+
| UnsafeWebViewFetch.swift:206:17:206:31 | call to getRemoteData() : | UnsafeWebViewFetch.swift:211:25:211:25 | htmlData |
34+
nodes
35+
| UnsafeWebViewFetch.swift:94:10:94:37 | try ... : | semmle.label | try ... : |
36+
| UnsafeWebViewFetch.swift:94:14:94:37 | call to ... : | semmle.label | call to ... : |
37+
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | semmle.label | call to getRemoteData() : |
38+
| UnsafeWebViewFetch.swift:120:25:120:39 | call to getRemoteData() | semmle.label | call to getRemoteData() |
39+
| UnsafeWebViewFetch.swift:121:25:121:25 | remoteString | semmle.label | remoteString |
40+
| UnsafeWebViewFetch.swift:124:25:124:51 | ... call to +(_:_:) ... | semmle.label | ... call to +(_:_:) ... |
41+
| UnsafeWebViewFetch.swift:135:25:135:25 | remoteString | semmle.label | remoteString |
42+
| UnsafeWebViewFetch.swift:137:25:137:25 | remoteString | semmle.label | remoteString |
43+
| UnsafeWebViewFetch.swift:138:47:138:56 | ...! | semmle.label | ...! |
44+
| UnsafeWebViewFetch.swift:139:25:139:25 | remoteString | semmle.label | remoteString |
45+
| UnsafeWebViewFetch.swift:139:48:139:57 | ...! | semmle.label | ...! |
46+
| UnsafeWebViewFetch.swift:140:47:140:57 | ...! | semmle.label | ...! |
47+
| UnsafeWebViewFetch.swift:141:25:141:25 | remoteString | semmle.label | remoteString |
48+
| UnsafeWebViewFetch.swift:141:48:141:58 | ...! | semmle.label | ...! |
49+
| UnsafeWebViewFetch.swift:153:85:153:94 | ...! | semmle.label | ...! |
50+
| UnsafeWebViewFetch.swift:154:86:154:95 | ...! | semmle.label | ...! |
51+
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | semmle.label | call to getRemoteData() : |
52+
| UnsafeWebViewFetch.swift:167:25:167:39 | call to getRemoteData() | semmle.label | call to getRemoteData() |
53+
| UnsafeWebViewFetch.swift:168:25:168:25 | remoteString | semmle.label | remoteString |
54+
| UnsafeWebViewFetch.swift:171:25:171:51 | ... call to +(_:_:) ... | semmle.label | ... call to +(_:_:) ... |
55+
| UnsafeWebViewFetch.swift:182:25:182:25 | remoteString | semmle.label | remoteString |
56+
| UnsafeWebViewFetch.swift:184:25:184:25 | remoteString | semmle.label | remoteString |
57+
| UnsafeWebViewFetch.swift:185:47:185:56 | ...! | semmle.label | ...! |
58+
| UnsafeWebViewFetch.swift:186:25:186:25 | remoteString | semmle.label | remoteString |
59+
| UnsafeWebViewFetch.swift:186:48:186:57 | ...! | semmle.label | ...! |
60+
| UnsafeWebViewFetch.swift:187:47:187:57 | ...! | semmle.label | ...! |
61+
| UnsafeWebViewFetch.swift:188:25:188:25 | remoteString | semmle.label | remoteString |
62+
| UnsafeWebViewFetch.swift:188:48:188:58 | ...! | semmle.label | ...! |
63+
| UnsafeWebViewFetch.swift:200:90:200:99 | ...! | semmle.label | ...! |
64+
| UnsafeWebViewFetch.swift:201:91:201:100 | ...! | semmle.label | ...! |
65+
| UnsafeWebViewFetch.swift:206:17:206:31 | call to getRemoteData() : | semmle.label | call to getRemoteData() : |
66+
| UnsafeWebViewFetch.swift:210:25:210:25 | htmlData | semmle.label | htmlData |
67+
| UnsafeWebViewFetch.swift:211:25:211:25 | htmlData | semmle.label | htmlData |
68+
subpaths
69+
#select
70+
| 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. |
71+
| 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. |
72+
| 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. |
73+
| 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. |
74+
| 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. |
75+
| 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. |
76+
| 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. |
77+
| 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. |
78+
| 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. |
79+
| 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. |
80+
| UnsafeWebViewFetch.swift:210:25:210:25 | htmlData | UnsafeWebViewFetch.swift:94:14:94:37 | call to ... : | UnsafeWebViewFetch.swift:210:25:210:25 | htmlData | Tainted data is used in a WebView fetch without restricting the base URL. |

‎swift/ql/test/query-tests/Security/CWE-079/UnsafeWebViewFetch.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
queries/Security/CWE-079/UnsafeWebViewFetch.ql

0 commit comments

Comments
 (0)