Skip to content

Commit 6a946f6

Browse files
geoffw0geoffw0
committed
Swift: Modernize.
1 parent 78eff0d commit 6a946f6

File tree

8 files changed

+175
-85
lines changed

8 files changed

+175
-85
lines changed

‎swift/ql/lib/codeql/swift/security/SqlInjectionExtensions.qll

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,27 @@ import swift
88
import codeql.swift.dataflow.DataFlow
99

1010
/**
11-
* A `DataFlow::Node` that is a sink for a SQL string to be executed.
11+
* A dataflow sink for SQL injection vulnerabilities.
1212
*/
13-
abstract class SqlSink extends DataFlow::Node { }
13+
abstract class SqlInjectionSink extends DataFlow::Node { }
1414

1515
/**
16-
* A sink for the sqlite3 C API.
16+
* A sanitizer for SQL injection vulnerabilities.
1717
*/
18-
class CApiSqlSink extends SqlSink {
19-
CApiSqlSink() {
18+
abstract class SqlInjectionSanitizer extends DataFlow::Node { }
19+
20+
/**
21+
* A unit class for adding additional taint steps.
22+
*/
23+
class SqlInjectionAdditionalTaintStep extends Unit {
24+
abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
25+
}
26+
27+
/**
28+
* A default SQL injection sink for the sqlite3 C API.
29+
*/
30+
class CApiDefaultSqlInjectionSink extends SqlInjectionSink {
31+
CApiDefaultSqlInjectionSink() {
2032
// `sqlite3_exec` and variants of `sqlite3_prepare`.
2133
exists(CallExpr call |
2234
call.getStaticTarget()
@@ -33,10 +45,10 @@ class CApiSqlSink extends SqlSink {
3345
}
3446

3547
/**
36-
* A sink for the SQLite.swift library.
48+
* A default SQL injection sink for the `SQLite.swift` library.
3749
*/
38-
class SQLiteSwiftSqlSink extends SqlSink {
39-
SQLiteSwiftSqlSink() {
50+
class SQLiteSwiftDefaultSqlInjectionSink extends SqlInjectionSink {
51+
SQLiteSwiftDefaultSqlInjectionSink() {
4052
// Variants of `Connection.execute`, `connection.prepare` and `connection.scalar`.
4153
exists(CallExpr call |
4254
call.getStaticTarget()
@@ -54,9 +66,11 @@ class SQLiteSwiftSqlSink extends SqlSink {
5466
}
5567
}
5668

57-
/** A sink for the GRDB library. */
58-
class GrdbSqlSink extends SqlSink {
59-
GrdbSqlSink() {
69+
/**
70+
* A default SQL injection sink for the GRDB library.
71+
*/
72+
class GrdbDefaultSqlInjectionSink extends SqlInjectionSink {
73+
GrdbDefaultSqlInjectionSink() {
6074
exists(CallExpr call, MethodDecl method |
6175
call.getStaticTarget() = method and
6276
call.getArgument(0).getExpr() = this.asExpr()

‎swift/ql/lib/codeql/swift/security/SqlInjectionQuery.qll

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,13 @@ class SqlInjectionConfig extends TaintTracking::Configuration {
1818

1919
override predicate isSource(DataFlow::Node node) { node instanceof FlowSource }
2020

21-
override predicate isSink(DataFlow::Node node) { node instanceof SqlSink }
21+
override predicate isSink(DataFlow::Node node) { node instanceof SqlInjectionSink }
22+
23+
override predicate isSanitizer(DataFlow::Node sanitizer) {
24+
sanitizer instanceof SqlInjectionSanitizer
25+
}
26+
27+
override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
28+
any(SqlInjectionAdditionalTaintStep s).step(nodeFrom, nodeTo)
29+
}
2230
}

‎swift/ql/lib/codeql/swift/security/UnsafeJsEvalExtensions.qll

Lines changed: 84 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,27 @@ import codeql.swift.dataflow.DataFlow
88
import codeql.swift.dataflow.FlowSources
99

1010
/**
11-
* A source of untrusted, user-controlled data.
11+
* A dataflow sink for javascript evaluation vulnerabilities.
1212
*/
13-
class Source = FlowSource;
13+
abstract class UnsafeJsEvalSink extends DataFlow::Node { }
1414

1515
/**
16-
* A sink that evaluates a string of JavaScript code.
16+
* A sanitizer for javascript evaluation vulnerabilities.
1717
*/
18-
abstract class Sink extends DataFlow::Node { }
18+
abstract class UnsafeJsEvalSanitizer extends DataFlow::Node { }
1919

20-
class WKWebView extends Sink {
21-
WKWebView() {
20+
/**
21+
* A unit class for adding additional taint steps.
22+
*/
23+
class UnsafeJsEvalAdditionalTaintStep extends Unit {
24+
abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
25+
}
26+
27+
/**
28+
* A default SQL injection sink for the `WKWebView` interface.
29+
*/
30+
class WKWebViewDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
31+
WKWebViewDefaultUnsafeJsEvalSink() {
2232
any(CallExpr ce |
2333
ce.getStaticTarget()
2434
.(MethodDecl)
@@ -34,8 +44,11 @@ class WKWebView extends Sink {
3444
}
3545
}
3646

37-
class WKUserContentController extends Sink {
38-
WKUserContentController() {
47+
/**
48+
* A default SQL injection sink for the `WKUserContentController` interface.
49+
*/
50+
class WKUserContentControllerDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
51+
WKUserContentControllerDefaultUnsafeJsEvalSink() {
3952
any(CallExpr ce |
4053
ce.getStaticTarget()
4154
.(MethodDecl)
@@ -44,8 +57,11 @@ class WKUserContentController extends Sink {
4457
}
4558
}
4659

47-
class UIWebView extends Sink {
48-
UIWebView() {
60+
/**
61+
* A default SQL injection sink for the `UIWebView` and `WebView` interfaces.
62+
*/
63+
class UIWebViewDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
64+
UIWebViewDefaultUnsafeJsEvalSink() {
4965
any(CallExpr ce |
5066
ce.getStaticTarget()
5167
.(MethodDecl)
@@ -54,8 +70,11 @@ class UIWebView extends Sink {
5470
}
5571
}
5672

57-
class JSContext extends Sink {
58-
JSContext() {
73+
/**
74+
* A default SQL injection sink for the `JSContext` interface.
75+
*/
76+
class JSContextDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
77+
JSContextDefaultUnsafeJsEvalSink() {
5978
any(CallExpr ce |
6079
ce.getStaticTarget()
6180
.(MethodDecl)
@@ -64,10 +83,61 @@ class JSContext extends Sink {
6483
}
6584
}
6685

67-
class JSEvaluateScript extends Sink {
68-
JSEvaluateScript() {
86+
/**
87+
* A default SQL injection sink for the `JSEvaluateScript` function.
88+
*/
89+
class JSEvaluateScriptDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
90+
JSEvaluateScriptDefaultUnsafeJsEvalSink() {
6991
any(CallExpr ce |
7092
ce.getStaticTarget().(FreeFunctionDecl).hasName("JSEvaluateScript(_:_:_:_:_:_:)")
7193
).getArgument(1).getExpr() = this.asExpr()
7294
}
7395
}
96+
97+
/**
98+
* A default SQL injection sanitrizer.
99+
*/
100+
class DefaultUnsafeJsEvalAdditionalTaintStep extends UnsafeJsEvalAdditionalTaintStep {
101+
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
102+
exists(Argument arg |
103+
arg =
104+
any(CallExpr ce |
105+
ce.getStaticTarget().(MethodDecl).hasQualifiedName("String", "init(decoding:as:)")
106+
).getArgument(0)
107+
or
108+
arg =
109+
any(CallExpr ce |
110+
ce.getStaticTarget()
111+
.(FreeFunctionDecl)
112+
.hasName([
113+
"JSStringCreateWithUTF8CString(_:)", "JSStringCreateWithCharacters(_:_:)",
114+
"JSStringRetain(_:)"
115+
])
116+
).getArgument(0)
117+
|
118+
nodeFrom.asExpr() = arg.getExpr() and
119+
nodeTo.asExpr() = arg.getApplyExpr()
120+
)
121+
or
122+
exists(CallExpr ce, Expr self, AbstractClosureExpr closure |
123+
ce.getStaticTarget()
124+
.getName()
125+
.matches(["withContiguousStorageIfAvailable(%)", "withUnsafeBufferPointer(%)"]) and
126+
self = ce.getQualifier() and
127+
ce.getArgument(0).getExpr() = closure
128+
|
129+
nodeFrom.asExpr() = self and
130+
nodeTo.(DataFlow::ParameterNode).getParameter() = closure.getParam(0)
131+
)
132+
or
133+
exists(MemberRefExpr e, Expr self, VarDecl member |
134+
self.getType().getName().matches(["Unsafe%Buffer%", "Unsafe%Pointer%"]) and
135+
member.getName() = "baseAddress"
136+
|
137+
e.getBase() = self and
138+
e.getMember() = member and
139+
nodeFrom.asExpr() = self and
140+
nodeTo.asExpr() = e
141+
)
142+
}
143+
}

‎swift/ql/lib/codeql/swift/security/UnsafeJsEvalQuery.qll

Lines changed: 7 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -15,51 +15,15 @@ import codeql.swift.security.UnsafeJsEvalExtensions
1515
class UnsafeJsEvalConfig extends TaintTracking::Configuration {
1616
UnsafeJsEvalConfig() { this = "UnsafeJsEvalConfig" }
1717

18-
override predicate isSource(DataFlow::Node node) { node instanceof Source }
18+
override predicate isSource(DataFlow::Node node) { node instanceof FlowSource }
1919

20-
override predicate isSink(DataFlow::Node node) { node instanceof Sink }
20+
override predicate isSink(DataFlow::Node node) { node instanceof UnsafeJsEvalSink }
21+
22+
override predicate isSanitizer(DataFlow::Node sanitizer) {
23+
sanitizer instanceof UnsafeJsEvalSanitizer
24+
}
2125

22-
// TODO: convert to new taint flow models
2326
override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
24-
exists(Argument arg |
25-
arg =
26-
any(CallExpr ce |
27-
ce.getStaticTarget().(MethodDecl).hasQualifiedName("String", "init(decoding:as:)")
28-
).getArgument(0)
29-
or
30-
arg =
31-
any(CallExpr ce |
32-
ce.getStaticTarget()
33-
.(FreeFunctionDecl)
34-
.hasName([
35-
"JSStringCreateWithUTF8CString(_:)", "JSStringCreateWithCharacters(_:_:)",
36-
"JSStringRetain(_:)"
37-
])
38-
).getArgument(0)
39-
|
40-
nodeFrom.asExpr() = arg.getExpr() and
41-
nodeTo.asExpr() = arg.getApplyExpr()
42-
)
43-
or
44-
exists(CallExpr ce, Expr self, AbstractClosureExpr closure |
45-
ce.getStaticTarget()
46-
.getName()
47-
.matches(["withContiguousStorageIfAvailable(%)", "withUnsafeBufferPointer(%)"]) and
48-
self = ce.getQualifier() and
49-
ce.getArgument(0).getExpr() = closure
50-
|
51-
nodeFrom.asExpr() = self and
52-
nodeTo.(DataFlow::ParameterNode).getParameter() = closure.getParam(0)
53-
)
54-
or
55-
exists(MemberRefExpr e, Expr self, VarDecl member |
56-
self.getType().getName().matches(["Unsafe%Buffer%", "Unsafe%Pointer%"]) and
57-
member.getName() = "baseAddress"
58-
|
59-
e.getBase() = self and
60-
e.getMember() = member and
61-
nodeFrom.asExpr() = self and
62-
nodeTo.asExpr() = e
63-
)
27+
any(UnsafeJsEvalAdditionalTaintStep s).step(nodeFrom, nodeTo)
6428
}
6529
}

‎swift/ql/lib/codeql/swift/security/UnsafeWebViewFetchExtensions.qll

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,36 @@ import swift
77
import codeql.swift.dataflow.DataFlow
88

99
/**
10-
* A sink that is a candidate result for this query, such as certain arguments
10+
* A dataflow sink for unsafe webview fetch vulnerabilities.
11+
*/
12+
abstract class UnsafeWebViewFetchSink extends DataFlow::Node {
13+
/**
14+
* Gets the `baseURL` argument associated with this sink (if any). These arguments affect
15+
* the way sinks are reported and are also sinks themselves.
16+
*/
17+
Expr getBaseUrl() { none() }
18+
}
19+
20+
/**
21+
* A sanitizer for unsafe webview fetch vulnerabilities.
22+
*/
23+
abstract class UnsafeWebViewFetchSanitizer extends DataFlow::Node { }
24+
25+
/**
26+
* A unit class for adding additional taint steps.
27+
*/
28+
class UnsafeWebViewFetchAdditionalTaintStep extends Unit {
29+
abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
30+
}
31+
32+
/**
33+
* A default uncontrolled format string sink, such as certain arguments
1134
* to `UIWebView.loadHTMLString`.
1235
*/
13-
class Sink extends DataFlow::Node {
36+
class DefaultUnsafeWebViewFetchSink extends UnsafeWebViewFetchSink {
1437
Expr baseUrl;
1538

16-
Sink() {
39+
DefaultUnsafeWebViewFetchSink() {
1740
exists(
1841
MethodDecl funcDecl, CallExpr call, string className, string funcName, int arg, int baseArg
1942
|
@@ -38,16 +61,13 @@ class Sink extends DataFlow::Node {
3861
baseArg = 3
3962
) and
4063
call.getStaticTarget() = funcDecl and
41-
// match up `funcName`, `paramName`, `arg`, `node`.
64+
// match up `className`, `funcName`.
4265
funcDecl.hasQualifiedName(className, funcName) and
43-
call.getArgument(arg).getExpr() = this.asExpr() and
44-
// match up `baseURLArg`
45-
call.getArgument(baseArg).getExpr() = baseUrl
66+
// match up `this`, `baseURL`
67+
this.asExpr() = call.getArgument(arg).getExpr() and // URL
68+
baseUrl = call.getArgument(baseArg).getExpr() // baseURL
4669
)
4770
}
4871

49-
/**
50-
* Gets the `baseURL` argument associated with this sink.
51-
*/
52-
Expr getBaseUrl() { result = baseUrl }
72+
override Expr getBaseUrl() { result = baseUrl }
5373
}

‎swift/ql/lib/codeql/swift/security/UnsafeWebViewFetchQuery.qll

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,17 @@ class UnsafeWebViewFetchConfig extends TaintTracking::Configuration {
1919
override predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
2020

2121
override predicate isSink(DataFlow::Node node) {
22-
node instanceof Sink or
23-
node.asExpr() = any(Sink s).getBaseUrl()
22+
exists(UnsafeWebViewFetchSink sink |
23+
node = sink or
24+
node.asExpr() = sink.getBaseUrl()
25+
)
26+
}
27+
28+
override predicate isSanitizer(DataFlow::Node sanitizer) {
29+
sanitizer instanceof UnsafeWebViewFetchSanitizer
30+
}
31+
32+
override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
33+
any(UnsafeWebViewFetchAdditionalTaintStep s).step(nodeFrom, nodeTo)
2434
}
2535
}

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,20 @@ import DataFlow::PathGraph
1919

2020
from
2121
UnsafeWebViewFetchConfig config, DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode,
22-
Sink sink, string message
22+
UnsafeWebViewFetchSink sink, string message
2323
where
2424
config.hasFlowPath(sourceNode, sinkNode) and
2525
sink = sinkNode.getNode() and
2626
(
27+
// no base URL
28+
not exists(sink.getBaseUrl()) and
29+
message = "Tainted data is used in a WebView fetch."
30+
or
2731
// base URL is nil
2832
sink.getBaseUrl() instanceof NilLiteralExpr and
2933
message = "Tainted data is used in a WebView fetch without restricting the base URL."
3034
or
31-
// base URL is tainted
35+
// base URL is also tainted
3236
config.hasFlowToExpr(sink.getBaseUrl()) and
3337
message = "Tainted data is used in a WebView fetch with a tainted base URL."
3438
)

‎swift/ql/src/queries/Security/CWE-094/UnsafeJsEval.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import codeql.swift.security.UnsafeJsEvalQuery
1818
import DataFlow::PathGraph
1919

2020
from
21-
UnsafeJsEvalConfig config, DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode, Sink sink
21+
UnsafeJsEvalConfig config, DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode, UnsafeJsEvalSink sink
2222
where
2323
config.hasFlowPath(sourceNode, sinkNode) and
2424
sink = sinkNode.getNode()

0 commit comments

Comments
 (0)