Skip to content

Commit 6783707

Browse files
geoffw0geoffw0
committed
Swift: Add heuristic sink. We don't catch everything, but the simple heuristic was better than anything else I tried.
1 parent 80cfb93 commit 6783707

File tree

3 files changed

+47
-6
lines changed

3 files changed

+47
-6
lines changed

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,26 @@ private class GrdbDefaultSqlInjectionSink extends SqlInjectionSink {
147147
}
148148
}
149149

150+
/**
151+
* An SQL injection sink that is determined by imprecise methods.
152+
*/
153+
private class HeuristicSqlInjectionSink extends SqlInjectionSink {
154+
HeuristicSqlInjectionSink() {
155+
// by parameter name
156+
exists(CallExpr ce, int ix, ParamDecl pd |
157+
pd.getName() = "sql" and
158+
pd = ce.getStaticTarget().getParam(ix) and
159+
this.asExpr() = ce.getArgument(ix).getExpr()
160+
)
161+
or
162+
// by argument name
163+
exists(Argument a |
164+
a.getLabel() = "sql" and
165+
this.asExpr() = a.getExpr()
166+
)
167+
}
168+
}
169+
150170
/**
151171
* A sink defined in a CSV model.
152172
*/

‎swift/ql/test/query-tests/Security/CWE-089/SqlInjection.expected

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,13 @@ edges
9797
| SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | SQLite.swift:117:16:117:16 | unsafeQuery1 |
9898
| SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | SQLite.swift:119:16:119:16 | unsafeQuery1 |
9999
| SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | SQLite.swift:132:20:132:20 | remoteString |
100+
| other.swift:46:25:46:79 | call to String.init(contentsOf:) | other.swift:50:22:50:22 | remoteString |
101+
| other.swift:46:25:46:79 | call to String.init(contentsOf:) | other.swift:52:14:52:14 | remoteString |
102+
| other.swift:46:25:46:79 | call to String.init(contentsOf:) | other.swift:53:14:53:14 | remoteString |
103+
| other.swift:46:25:46:79 | call to String.init(contentsOf:) | other.swift:54:31:54:31 | remoteString |
104+
| other.swift:46:25:46:79 | call to String.init(contentsOf:) | other.swift:55:14:55:14 | remoteString |
105+
| other.swift:46:25:46:79 | call to String.init(contentsOf:) | other.swift:57:16:57:16 | remoteString |
106+
| other.swift:54:31:54:31 | remoteString | other.swift:54:14:54:43 | call to NSString.init(string:) |
100107
| sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | sqlite3_c_api.swift:133:33:133:33 | unsafeQuery1 |
101108
| sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | sqlite3_c_api.swift:134:33:134:33 | unsafeQuery2 |
102109
| sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | sqlite3_c_api.swift:135:33:135:33 | unsafeQuery3 |
@@ -222,6 +229,14 @@ nodes
222229
| SQLite.swift:117:16:117:16 | unsafeQuery1 | semmle.label | unsafeQuery1 |
223230
| SQLite.swift:119:16:119:16 | unsafeQuery1 | semmle.label | unsafeQuery1 |
224231
| SQLite.swift:132:20:132:20 | remoteString | semmle.label | remoteString |
232+
| other.swift:46:25:46:79 | call to String.init(contentsOf:) | semmle.label | call to String.init(contentsOf:) |
233+
| other.swift:50:22:50:22 | remoteString | semmle.label | remoteString |
234+
| other.swift:52:14:52:14 | remoteString | semmle.label | remoteString |
235+
| other.swift:53:14:53:14 | remoteString | semmle.label | remoteString |
236+
| other.swift:54:14:54:43 | call to NSString.init(string:) | semmle.label | call to NSString.init(string:) |
237+
| other.swift:54:31:54:31 | remoteString | semmle.label | remoteString |
238+
| other.swift:55:14:55:14 | remoteString | semmle.label | remoteString |
239+
| other.swift:57:16:57:16 | remoteString | semmle.label | remoteString |
225240
| sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | semmle.label | call to String.init(contentsOf:) |
226241
| sqlite3_c_api.swift:133:33:133:33 | unsafeQuery1 | semmle.label | unsafeQuery1 |
227242
| sqlite3_c_api.swift:134:33:134:33 | unsafeQuery2 | semmle.label | unsafeQuery2 |
@@ -336,6 +351,12 @@ subpaths
336351
| SQLite.swift:117:16:117:16 | unsafeQuery1 | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | SQLite.swift:117:16:117:16 | unsafeQuery1 | This query depends on a $@. | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | user-provided value |
337352
| SQLite.swift:119:16:119:16 | unsafeQuery1 | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | SQLite.swift:119:16:119:16 | unsafeQuery1 | This query depends on a $@. | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | user-provided value |
338353
| SQLite.swift:132:20:132:20 | remoteString | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | SQLite.swift:132:20:132:20 | remoteString | This query depends on a $@. | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | user-provided value |
354+
| other.swift:50:22:50:22 | remoteString | other.swift:46:25:46:79 | call to String.init(contentsOf:) | other.swift:50:22:50:22 | remoteString | This query depends on a $@. | other.swift:46:25:46:79 | call to String.init(contentsOf:) | user-provided value |
355+
| other.swift:52:14:52:14 | remoteString | other.swift:46:25:46:79 | call to String.init(contentsOf:) | other.swift:52:14:52:14 | remoteString | This query depends on a $@. | other.swift:46:25:46:79 | call to String.init(contentsOf:) | user-provided value |
356+
| other.swift:53:14:53:14 | remoteString | other.swift:46:25:46:79 | call to String.init(contentsOf:) | other.swift:53:14:53:14 | remoteString | This query depends on a $@. | other.swift:46:25:46:79 | call to String.init(contentsOf:) | user-provided value |
357+
| other.swift:54:14:54:43 | call to NSString.init(string:) | other.swift:46:25:46:79 | call to String.init(contentsOf:) | other.swift:54:14:54:43 | call to NSString.init(string:) | This query depends on a $@. | other.swift:46:25:46:79 | call to String.init(contentsOf:) | user-provided value |
358+
| other.swift:55:14:55:14 | remoteString | other.swift:46:25:46:79 | call to String.init(contentsOf:) | other.swift:55:14:55:14 | remoteString | This query depends on a $@. | other.swift:46:25:46:79 | call to String.init(contentsOf:) | user-provided value |
359+
| other.swift:57:16:57:16 | remoteString | other.swift:46:25:46:79 | call to String.init(contentsOf:) | other.swift:57:16:57:16 | remoteString | This query depends on a $@. | other.swift:46:25:46:79 | call to String.init(contentsOf:) | user-provided value |
339360
| sqlite3_c_api.swift:133:33:133:33 | unsafeQuery1 | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | sqlite3_c_api.swift:133:33:133:33 | unsafeQuery1 | This query depends on a $@. | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | user-provided value |
340361
| sqlite3_c_api.swift:134:33:134:33 | unsafeQuery2 | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | sqlite3_c_api.swift:134:33:134:33 | unsafeQuery2 | This query depends on a $@. | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | user-provided value |
341362
| sqlite3_c_api.swift:135:33:135:33 | unsafeQuery3 | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | sqlite3_c_api.swift:135:33:135:33 | unsafeQuery3 | This query depends on a $@. | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | user-provided value |

‎swift/ql/test/query-tests/Security/CWE-089/other.swift

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,14 @@ func test_heuristic(db: MyDatabase) throws {
4747

4848
_ = MyDatabase()
4949
_ = MyDatabase(sql: "some_fixed_sql")
50-
_ = MyDatabase(sql: remoteString) // BAD [NOT DETECTED]
50+
_ = MyDatabase(sql: remoteString) // BAD
5151

52-
db.execute1(remoteString) // BAD [NOT DETECTED]
53-
db.execute2(remoteString) // BAD [NOT DETECTED]
54-
db.execute3(NSString(string: remoteString)) // BAD [NOT DETECTED]
55-
db.execute4(remoteString as! Sql) // BAD [NOT DETECTED]
52+
db.execute1(remoteString) // BAD
53+
db.execute2(remoteString) // BAD
54+
db.execute3(NSString(string: remoteString)) // BAD
55+
db.execute4(remoteString as! Sql) // BAD
5656

57-
db.query(sql: remoteString) // BAD [NOT DETECTED]
57+
db.query(sql: remoteString) // BAD
5858
db.query(sqlLiteral: remoteString) // BAD [NOT DETECTED]
5959
db.query(sqlStatement: remoteString) // BAD [NOT DETECTED]
6060
db.query(sqliteStatement: remoteString) // BAD [NOT DETECTED]

0 commit comments

Comments
 (0)