Skip to content

Commit ee442e4

Browse files
geoffw0geoffw0
authored
Merge pull request github#11979 from geoffw0/modern1
Swift: Modernize injection queries
2 parents cd59640 + fe13137 commit ee442e4

File tree

9 files changed

+491
-322
lines changed

9 files changed

+491
-322
lines changed

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

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
/**
2+
* Provides classes and predicates for reasoning about database
3+
* queries built from user-controlled sources (that is, SQL injection
4+
* vulnerabilities).
5+
*/
6+
7+
import swift
8+
import codeql.swift.dataflow.DataFlow
9+
private import codeql.swift.dataflow.ExternalFlow
10+
11+
/**
12+
* A dataflow sink for SQL injection vulnerabilities.
13+
*/
14+
abstract class SqlInjectionSink extends DataFlow::Node { }
15+
16+
/**
17+
* A sanitizer for SQL injection vulnerabilities.
18+
*/
19+
abstract class SqlInjectionSanitizer extends DataFlow::Node { }
20+
21+
/**
22+
* A unit class for adding additional taint steps.
23+
*/
24+
class SqlInjectionAdditionalTaintStep extends Unit {
25+
abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
26+
}
27+
28+
/**
29+
* A default SQL injection sink for the sqlite3 C API.
30+
*/
31+
private class CApiDefaultSqlInjectionSink extends SqlInjectionSink {
32+
CApiDefaultSqlInjectionSink() {
33+
// `sqlite3_exec` and variants of `sqlite3_prepare`.
34+
exists(CallExpr call |
35+
call.getStaticTarget()
36+
.(FreeFunctionDecl)
37+
.hasName([
38+
"sqlite3_exec(_:_:_:_:_:)", "sqlite3_prepare(_:_:_:_:_:)",
39+
"sqlite3_prepare_v2(_:_:_:_:_:)", "sqlite3_prepare_v3(_:_:_:_:_:_:)",
40+
"sqlite3_prepare16(_:_:_:_:_:)", "sqlite3_prepare16_v2(_:_:_:_:_:)",
41+
"sqlite3_prepare16_v3(_:_:_:_:_:_:)"
42+
]) and
43+
call.getArgument(1).getExpr() = this.asExpr()
44+
)
45+
}
46+
}
47+
48+
/**
49+
* A default SQL injection sink for the `SQLite.swift` library.
50+
*/
51+
private class SQLiteSwiftDefaultSqlInjectionSink extends SqlInjectionSink {
52+
SQLiteSwiftDefaultSqlInjectionSink() {
53+
// Variants of `Connection.execute`, `connection.prepare` and `connection.scalar`.
54+
exists(CallExpr call |
55+
call.getStaticTarget()
56+
.(MethodDecl)
57+
.hasQualifiedName("Connection",
58+
["execute(_:)", "prepare(_:_:)", "run(_:_:)", "scalar(_:_:)"]) and
59+
call.getArgument(0).getExpr() = this.asExpr()
60+
)
61+
or
62+
// String argument to the `Statement` constructor.
63+
exists(CallExpr call |
64+
call.getStaticTarget().(MethodDecl).hasQualifiedName("Statement", "init(_:_:)") and
65+
call.getArgument(1).getExpr() = this.asExpr()
66+
)
67+
}
68+
}
69+
70+
/**
71+
* A default SQL injection sink for the GRDB library.
72+
*/
73+
private class GrdbDefaultSqlInjectionSink extends SqlInjectionSink {
74+
GrdbDefaultSqlInjectionSink() {
75+
exists(CallExpr call, MethodDecl method |
76+
call.getStaticTarget() = method and
77+
call.getArgument(0).getExpr() = this.asExpr()
78+
|
79+
method
80+
.hasQualifiedName("Database",
81+
[
82+
"allStatements(sql:arguments:)", "cachedStatement(sql:)",
83+
"internalCachedStatement(sql:)", "execute(sql:arguments:)", "makeStatement(sql:)",
84+
"makeStatement(sql:prepFlags:)"
85+
])
86+
or
87+
method
88+
.hasQualifiedName("SQLRequest",
89+
[
90+
"init(stringLiteral:)", "init(unicodeScalarLiteral:)",
91+
"init(extendedGraphemeClusterLiteral:)", "init(stringInterpolation:)",
92+
"init(sql:arguments:adapter:cached:)"
93+
])
94+
or
95+
method
96+
.hasQualifiedName("SQL",
97+
[
98+
"init(stringLiteral:)", "init(unicodeScalarLiteral:)",
99+
"init(extendedGraphemeClusterLiteral:)", "init(stringInterpolation:)",
100+
"init(sql:arguments:)", "append(sql:arguments:)"
101+
])
102+
or
103+
method
104+
.hasQualifiedName("TableDefinition", ["column(sql:)", "check(sql:)", "constraint(sql:)"])
105+
or
106+
method.hasQualifiedName("TableAlteration", "addColumn(sql:)")
107+
or
108+
method
109+
.hasQualifiedName("ColumnDefinition",
110+
["check(sql:)", "defaults(sql:)", "generatedAs(sql:_:)"])
111+
or
112+
method
113+
.hasQualifiedName("TableRecord",
114+
[
115+
"select(sql:arguments:)", "select(sql:arguments:as:)", "filter(sql:arguments:)",
116+
"order(sql:arguments:)"
117+
])
118+
or
119+
method.hasQualifiedName("StatementCache", "statement(_:)")
120+
)
121+
or
122+
exists(CallExpr call, MethodDecl method |
123+
call.getStaticTarget() = method and
124+
call.getArgument(1).getExpr() = this.asExpr()
125+
|
126+
method
127+
.hasQualifiedName(["Row", "DatabaseValueConvertible"],
128+
[
129+
"fetchCursor(_:sql:arguments:adapter:)", "fetchAll(_:sql:arguments:adapter:)",
130+
"fetchSet(_:sql:arguments:adapter:)", "fetchOne(_:sql:arguments:adapter:)"
131+
])
132+
or
133+
method.hasQualifiedName("SQLStatementCursor", "init(database:sql:arguments:prepFlags:)")
134+
)
135+
or
136+
exists(CallExpr call, MethodDecl method |
137+
call.getStaticTarget() = method and
138+
call.getArgument(3).getExpr() = this.asExpr()
139+
|
140+
method
141+
.hasQualifiedName("CommonTableExpression", "init(recursive:named:columns:sql:arguments:)")
142+
)
143+
}
144+
}
145+
146+
/**
147+
* A sink defined in a CSV model.
148+
*/
149+
private class DefaultSqlInjectionSink extends SqlInjectionSink {
150+
DefaultSqlInjectionSink() { sinkNode(this, "sql") }
151+
}

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/**
2+
* Provides a taint-tracking configuration for reasoning about database
3+
* queries built from user-controlled sources (that is, SQL injection
4+
* vulnerabilities).
5+
*/
6+
7+
import swift
8+
import codeql.swift.dataflow.DataFlow
9+
import codeql.swift.dataflow.TaintTracking
10+
import codeql.swift.dataflow.FlowSources
11+
import codeql.swift.security.SqlInjectionExtensions
12+
13+
/**
14+
* A taint configuration for tainted data that reaches a SQL sink.
15+
*/
16+
class SqlInjectionConfig extends TaintTracking::Configuration {
17+
SqlInjectionConfig() { this = "SqlInjectionConfig" }
18+
19+
override predicate isSource(DataFlow::Node node) { node instanceof FlowSource }
20+
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+
}
30+
}

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

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
/**
2+
* Provides classes and predicates for reasoning about javascript
3+
* evaluation vulnerabilities.
4+
*/
5+
6+
import swift
7+
import codeql.swift.dataflow.DataFlow
8+
import codeql.swift.dataflow.FlowSources
9+
private import codeql.swift.dataflow.ExternalFlow
10+
11+
/**
12+
* A dataflow sink for javascript evaluation vulnerabilities.
13+
*/
14+
abstract class UnsafeJsEvalSink extends DataFlow::Node { }
15+
16+
/**
17+
* A sanitizer for javascript evaluation vulnerabilities.
18+
*/
19+
abstract class UnsafeJsEvalSanitizer extends DataFlow::Node { }
20+
21+
/**
22+
* A unit class for adding additional taint steps.
23+
*/
24+
class UnsafeJsEvalAdditionalTaintStep extends Unit {
25+
abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
26+
}
27+
28+
/**
29+
* A default SQL injection sink for the `WKWebView` interface.
30+
*/
31+
private class WKWebViewDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
32+
WKWebViewDefaultUnsafeJsEvalSink() {
33+
any(CallExpr ce |
34+
ce.getStaticTarget()
35+
.(MethodDecl)
36+
.hasQualifiedName("WKWebView",
37+
[
38+
"evaluateJavaScript(_:)", "evaluateJavaScript(_:completionHandler:)",
39+
"evaluateJavaScript(_:in:in:completionHandler:)",
40+
"evaluateJavaScript(_:in:contentWorld:)",
41+
"callAsyncJavaScript(_:arguments:in:in:completionHandler:)",
42+
"callAsyncJavaScript(_:arguments:in:contentWorld:)"
43+
])
44+
).getArgument(0).getExpr() = this.asExpr()
45+
}
46+
}
47+
48+
/**
49+
* A default SQL injection sink for the `WKUserContentController` interface.
50+
*/
51+
private class WKUserContentControllerDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
52+
WKUserContentControllerDefaultUnsafeJsEvalSink() {
53+
any(CallExpr ce |
54+
ce.getStaticTarget()
55+
.(MethodDecl)
56+
.hasQualifiedName("WKUserContentController", "addUserScript(_:)")
57+
).getArgument(0).getExpr() = this.asExpr()
58+
}
59+
}
60+
61+
/**
62+
* A default SQL injection sink for the `UIWebView` and `WebView` interfaces.
63+
*/
64+
private class UIWebViewDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
65+
UIWebViewDefaultUnsafeJsEvalSink() {
66+
any(CallExpr ce |
67+
ce.getStaticTarget()
68+
.(MethodDecl)
69+
.hasQualifiedName(["UIWebView", "WebView"], "stringByEvaluatingJavaScript(from:)")
70+
).getArgument(0).getExpr() = this.asExpr()
71+
}
72+
}
73+
74+
/**
75+
* A default SQL injection sink for the `JSContext` interface.
76+
*/
77+
private class JSContextDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
78+
JSContextDefaultUnsafeJsEvalSink() {
79+
any(CallExpr ce |
80+
ce.getStaticTarget()
81+
.(MethodDecl)
82+
.hasQualifiedName("JSContext", ["evaluateScript(_:)", "evaluateScript(_:withSourceURL:)"])
83+
).getArgument(0).getExpr() = this.asExpr()
84+
}
85+
}
86+
87+
/**
88+
* A default SQL injection sink for the `JSEvaluateScript` function.
89+
*/
90+
private class JSEvaluateScriptDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
91+
JSEvaluateScriptDefaultUnsafeJsEvalSink() {
92+
any(CallExpr ce |
93+
ce.getStaticTarget().(FreeFunctionDecl).hasName("JSEvaluateScript(_:_:_:_:_:_:)")
94+
).getArgument(1).getExpr() = this.asExpr()
95+
}
96+
}
97+
98+
/**
99+
* A default SQL injection sanitrizer.
100+
*/
101+
private class DefaultUnsafeJsEvalAdditionalTaintStep extends UnsafeJsEvalAdditionalTaintStep {
102+
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
103+
exists(Argument arg |
104+
arg =
105+
any(CallExpr ce |
106+
ce.getStaticTarget().(MethodDecl).hasQualifiedName("String", "init(decoding:as:)")
107+
).getArgument(0)
108+
or
109+
arg =
110+
any(CallExpr ce |
111+
ce.getStaticTarget()
112+
.(FreeFunctionDecl)
113+
.hasName([
114+
"JSStringCreateWithUTF8CString(_:)", "JSStringCreateWithCharacters(_:_:)",
115+
"JSStringRetain(_:)"
116+
])
117+
).getArgument(0)
118+
|
119+
nodeFrom.asExpr() = arg.getExpr() and
120+
nodeTo.asExpr() = arg.getApplyExpr()
121+
)
122+
or
123+
exists(CallExpr ce, Expr self, AbstractClosureExpr closure |
124+
ce.getStaticTarget()
125+
.getName()
126+
.matches(["withContiguousStorageIfAvailable(%)", "withUnsafeBufferPointer(%)"]) and
127+
self = ce.getQualifier() and
128+
ce.getArgument(0).getExpr() = closure
129+
|
130+
nodeFrom.asExpr() = self and
131+
nodeTo.(DataFlow::ParameterNode).getParameter() = closure.getParam(0)
132+
)
133+
or
134+
exists(MemberRefExpr e, Expr self, VarDecl member |
135+
self.getType().getName().matches(["Unsafe%Buffer%", "Unsafe%Pointer%"]) and
136+
member.getName() = "baseAddress"
137+
|
138+
e.getBase() = self and
139+
e.getMember() = member and
140+
nodeFrom.asExpr() = self and
141+
nodeTo.asExpr() = e
142+
)
143+
}
144+
}
145+
146+
/**
147+
* A sink defined in a CSV model.
148+
*/
149+
private class DefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
150+
DefaultUnsafeJsEvalSink() { sinkNode(this, "js-eval") }
151+
}

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* Provides a taint-tracking configuration for reasoning about javascript
3+
* evaluation vulnerabilities.
4+
*/
5+
6+
import swift
7+
import codeql.swift.dataflow.DataFlow
8+
import codeql.swift.dataflow.TaintTracking
9+
import codeql.swift.dataflow.FlowSources
10+
import codeql.swift.security.UnsafeJsEvalExtensions
11+
12+
/**
13+
* A taint configuration from taint sources to sinks for this query.
14+
*/
15+
class UnsafeJsEvalConfig extends TaintTracking::Configuration {
16+
UnsafeJsEvalConfig() { this = "UnsafeJsEvalConfig" }
17+
18+
override predicate isSource(DataFlow::Node node) { node instanceof FlowSource }
19+
20+
override predicate isSink(DataFlow::Node node) { node instanceof UnsafeJsEvalSink }
21+
22+
override predicate isSanitizer(DataFlow::Node sanitizer) {
23+
sanitizer instanceof UnsafeJsEvalSanitizer
24+
}
25+
26+
override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
27+
any(UnsafeJsEvalAdditionalTaintStep s).step(nodeFrom, nodeTo)
28+
}
29+
}

0 commit comments

Comments
 (0)