Skip to content

Commit a4f8131

Browse files
atorralbaatorralba
authored
Merge pull request github#11785 from atorralba/atorralba/swift/grdb-sinks
Swift: Add sinks for the GRDB library
2 parents ce06df3 + eb78661 commit a4f8131

File tree

9 files changed

+1330
-56
lines changed

9 files changed

+1330
-56
lines changed

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,13 @@ private class PathInjectionSinks extends SinkModelCsv {
104104
";FileManager;true;replaceItemAtURL(originalItemURL:withItemAtURL:backupItemName:options:);;;Argument[0..1];path-injection",
105105
";NIOFileHandle;true;init(descriptor:);;;Argument[0];path-injection",
106106
";NIOFileHandle;true;init(path:mode:flags:);;;Argument[0];path-injection",
107-
";NIOFileHandle;true;init(path:);;;Argument[0];path-injection"
107+
";NIOFileHandle;true;init(path:);;;Argument[0];path-injection",
108+
// GRDB
109+
";Database;true;init(path:description:configuration:);;;Argument[0];path-injection",
110+
";DatabasePool;true;init(path:configuration:);;;Argument[0];path-injection",
111+
";DatabaseQueue;true;init(path:configuration:);;;Argument[0];path-injection",
112+
";DatabaseSnapshotPool;true;init(path:configuration:);;;Argument[0];path-injection",
113+
";SerializedDatabase;true;init(path:configuration:defaultLabel:purpose:);;;Argument[0];path-injection"
108114
]
109115
}
110116
}

‎swift/ql/src/queries/Security/CWE-089/SqlInjection.ql

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,80 @@ class SQLiteSwiftSqlSink extends SqlSink {
6363
}
6464
}
6565

66+
/** A sink for the GRDB library. */
67+
class GrdbSqlSink extends SqlSink {
68+
GrdbSqlSink() {
69+
exists(CallExpr call, MethodDecl method |
70+
call.getStaticTarget() = method and
71+
call.getArgument(0).getExpr() = this.asExpr()
72+
|
73+
method
74+
.hasQualifiedName("Database",
75+
[
76+
"allStatements(sql:arguments:)", "cachedStatement(sql:)",
77+
"internalCachedStatement(sql:)", "execute(sql:arguments:)", "makeStatement(sql:)",
78+
"makeStatement(sql:prepFlags:)"
79+
])
80+
or
81+
method
82+
.hasQualifiedName("SQLRequest",
83+
[
84+
"init(stringLiteral:)", "init(unicodeScalarLiteral:)",
85+
"init(extendedGraphemeClusterLiteral:)", "init(stringInterpolation:)",
86+
"init(sql:arguments:adapter:cached:)"
87+
])
88+
or
89+
method
90+
.hasQualifiedName("SQL",
91+
[
92+
"init(stringLiteral:)", "init(unicodeScalarLiteral:)",
93+
"init(extendedGraphemeClusterLiteral:)", "init(stringInterpolation:)",
94+
"init(sql:arguments:)", "append(sql:arguments:)"
95+
])
96+
or
97+
method
98+
.hasQualifiedName("TableDefinition", ["column(sql:)", "check(sql:)", "constraint(sql:)"])
99+
or
100+
method.hasQualifiedName("TableAlteration", "addColumn(sql:)")
101+
or
102+
method
103+
.hasQualifiedName("ColumnDefinition",
104+
["check(sql:)", "defaults(sql:)", "generatedAs(sql:_:)"])
105+
or
106+
method
107+
.hasQualifiedName("TableRecord",
108+
[
109+
"select(sql:arguments:)", "select(sql:arguments:as:)", "filter(sql:arguments:)",
110+
"order(sql:arguments:)"
111+
])
112+
or
113+
method.hasQualifiedName("StatementCache", "statement(_:)")
114+
)
115+
or
116+
exists(CallExpr call, MethodDecl method |
117+
call.getStaticTarget() = method and
118+
call.getArgument(1).getExpr() = this.asExpr()
119+
|
120+
method
121+
.hasQualifiedName(["Row", "DatabaseValueConvertible"],
122+
[
123+
"fetchCursor(_:sql:arguments:adapter:)", "fetchAll(_:sql:arguments:adapter:)",
124+
"fetchSet(_:sql:arguments:adapter:)", "fetchOne(_:sql:arguments:adapter:)"
125+
])
126+
or
127+
method.hasQualifiedName("SQLStatementCursor", "init(database:sql:arguments:prepFlags:)")
128+
)
129+
or
130+
exists(CallExpr call, MethodDecl method |
131+
call.getStaticTarget() = method and
132+
call.getArgument(3).getExpr() = this.asExpr()
133+
|
134+
method
135+
.hasQualifiedName("CommonTableExpression", "init(recursive:named:columns:sql:arguments:)")
136+
)
137+
}
138+
}
139+
66140
/**
67141
* A taint configuration for tainted data that reaches a SQL sink.
68142
*/

‎swift/ql/src/queries/Security/CWE-311/CleartextStorageDatabase.ql

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,61 @@ class RealmStore extends Stored instanceof DataFlow::PostUpdateNode {
5656
}
5757
}
5858

59+
/**
60+
* A `DataFlow::Node` that is an expression stored with the GRDB library.
61+
*/
62+
class GrdbStore extends Stored {
63+
GrdbStore() {
64+
exists(CallExpr call, MethodDecl method |
65+
call.getStaticTarget() = method and
66+
call.getArgumentWithLabel("arguments").getExpr() = this.asExpr()
67+
|
68+
method
69+
.hasQualifiedName("Database",
70+
["allStatements(sql:arguments:)", "execute(sql:arguments:)",])
71+
or
72+
method.hasQualifiedName("SQLRequest", "init(sql:arguments:adapter:cached:)")
73+
or
74+
method.hasQualifiedName("SQL", ["init(sql:arguments:)", "append(sql:arguments:)"])
75+
or
76+
method.hasQualifiedName("SQLStatementCursor", "init(database:sql:arguments:prepFlags:)")
77+
or
78+
method
79+
.hasQualifiedName("TableRecord",
80+
[
81+
"select(sql:arguments:)", "select(sql:arguments:as:)", "filter(sql:arguments:)",
82+
"order(sql:arguments:)"
83+
])
84+
or
85+
method
86+
.hasQualifiedName(["Row", "DatabaseValueConvertible", "FetchableRecord"],
87+
[
88+
"fetchCursor(_:sql:arguments:adapter:)", "fetchAll(_:sql:arguments:adapter:)",
89+
"fetchSet(_:sql:arguments:adapter:)", "fetchOne(_:sql:arguments:adapter:)"
90+
])
91+
or
92+
method
93+
.hasQualifiedName("FetchableRecord",
94+
[
95+
"fetchCursor(_:arguments:adapter:)", "fetchAll(_:arguments:adapter:)",
96+
"fetchSet(_:arguments:adapter:)", "fetchOne(_:arguments:adapter:)",
97+
])
98+
or
99+
method.hasQualifiedName("Statement", ["execute(arguments:)"])
100+
or
101+
method
102+
.hasQualifiedName("CommonTableExpression", "init(recursive:named:columns:sql:arguments:)")
103+
)
104+
or
105+
exists(CallExpr call, MethodDecl method |
106+
call.getStaticTarget() = method and
107+
call.getArgument(0).getExpr() = this.asExpr()
108+
|
109+
method.hasQualifiedName("Statement", "setArguments(_:)")
110+
)
111+
}
112+
}
113+
59114
/**
60115
* A taint configuration from sensitive information to expressions that are
61116
* transmitted over a network.
@@ -77,6 +132,14 @@ class CleartextStorageConfig extends TaintTracking::Configuration {
77132
node.asExpr() instanceof EncryptedExpr
78133
}
79134

135+
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
136+
// Needed until we have proper content flow through arrays
137+
exists(ArrayExpr arr |
138+
node1.asExpr() = arr.getAnElement() and
139+
node2.asExpr() = arr
140+
)
141+
}
142+
80143
override predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {
81144
// flow out from fields of a `RealmSwiftObject` at the sink, for example in
82145
// `realmObj.data = sensitive`.

‎swift/ql/test/query-tests/Security/CWE-022/testPathInjection.swift

Lines changed: 96 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,30 @@ class FileManager {
9191
func replaceItemAtURL(originalItemURL: NSURL, withItemAtURL: NSURL, backupItemName: String?, options: FileManager.ItemReplacementOptions) -> NSURL? { return nil }
9292
}
9393

94+
// GRDB
95+
96+
struct Configuration {}
97+
98+
class Database {
99+
init(path: String, description: String, configuration: Configuration) {}
100+
}
101+
102+
class DatabasePool {
103+
init(path: String, configuration: Configuration) {}
104+
}
105+
106+
class DatabaseQueue {
107+
init(path: String, configuration: Configuration) {}
108+
}
109+
110+
class DatabaseSnapshotPool {
111+
init(path: String, configuration: Configuration) {}
112+
}
113+
114+
class SerializedDatabase {
115+
init(path: String, configuration: Configuration = Configuration(), defaultLabel: String, purpose: String? = nil) {}
116+
}
117+
94118
// --- tests ---
95119

96120
func test() {
@@ -100,65 +124,82 @@ func test() {
100124
let safeUrl = URL(string: "")!
101125
let safeNsUrl = NSURL(string: "")!
102126

103-
Data("").write(to: remoteUrl, options: []) // $ hasPathInjection=97
127+
Data("").write(to: remoteUrl, options: []) // $ hasPathInjection=121
104128

105129
let nsData = NSData()
106-
let _ = nsData.write(to: remoteUrl, atomically: false) // $ hasPathInjection=97
107-
nsData.write(to: remoteUrl, options: []) // $ hasPathInjection=97
108-
let _ = nsData.write(toFile: remoteString, atomically: false) // $ hasPathInjection=97
109-
nsData.write(toFile: remoteString, options: []) // $ hasPathInjection=97
130+
let _ = nsData.write(to: remoteUrl, atomically: false) // $ hasPathInjection=121
131+
nsData.write(to: remoteUrl, options: []) // $ hasPathInjection=121
132+
let _ = nsData.write(toFile: remoteString, atomically: false) // $ hasPathInjection=121
133+
nsData.write(toFile: remoteString, options: []) // $ hasPathInjection=121
110134

111135
let fm = FileManager()
112-
let _ = fm.contentsOfDirectory(at: remoteUrl, includingPropertiesForKeys: [], options: []) // $ hasPathInjection=97
113-
let _ = fm.contentsOfDirectory(atPath: remoteString) // $ hasPathInjection=97
114-
let _ = fm.enumerator(at: remoteUrl, includingPropertiesForKeys: [], options: [], errorHandler: nil) // $ hasPathInjection=97
115-
let _ = fm.enumerator(atPath: remoteString) // $ hasPathInjection=97
116-
let _ = fm.subpathsOfDirectory(atPath: remoteString) // $ hasPathInjection=97
117-
let _ = fm.subpaths(atPath: remoteString) // $ hasPathInjection=97
118-
fm.createDirectory(at: remoteUrl, withIntermediateDirectories: false, attributes: [:]) // $ hasPathInjection=97
119-
let _ = fm.createDirectory(atPath: remoteString, attributes: [:]) // $ hasPathInjection=97
120-
let _ = fm.createFile(atPath: remoteString, contents: nil, attributes: [:]) // $ hasPathInjection=97
121-
fm.removeItem(at: remoteUrl) // $ hasPathInjection=97
122-
fm.removeItem(atPath: remoteString) // $ hasPathInjection=97
123-
fm.trashItem(at: remoteUrl, resultingItemURL: AutoreleasingUnsafeMutablePointer<NSURL?>()) // $ hasPathInjection=97
124-
let _ = fm.replaceItemAt(remoteUrl, withItemAt: safeUrl, backupItemName: nil, options: []) // $ hasPathInjection=97
125-
let _ = fm.replaceItemAt(safeUrl, withItemAt: remoteUrl, backupItemName: nil, options: []) // $ hasPathInjection=97
126-
fm.replaceItem(at: remoteUrl, withItemAt: safeUrl, backupItemName: nil, options: [], resultingItemURL: AutoreleasingUnsafeMutablePointer<NSURL?>()) // $ hasPathInjection=97
127-
fm.replaceItem(at: safeUrl, withItemAt: remoteUrl, backupItemName: nil, options: [], resultingItemURL: AutoreleasingUnsafeMutablePointer<NSURL?>()) // $ hasPathInjection=97
128-
fm.copyItem(at: remoteUrl, to: safeUrl) // $ hasPathInjection=97
129-
fm.copyItem(at: safeUrl, to: remoteUrl) // $ hasPathInjection=97
130-
fm.copyItem(atPath: remoteString, toPath: "") // $ hasPathInjection=97
131-
fm.copyItem(atPath: "", toPath: remoteString) // $ hasPathInjection=97
132-
fm.moveItem(at: remoteUrl, to: safeUrl) // $ hasPathInjection=97
133-
fm.moveItem(at: safeUrl, to: remoteUrl) // $ hasPathInjection=97
134-
fm.moveItem(atPath: remoteString, toPath: "") // $ hasPathInjection=97
135-
fm.moveItem(atPath: "", toPath: remoteString) // $ hasPathInjection=97
136-
fm.createSymbolicLink(at: remoteUrl, withDestinationURL: safeUrl) // $ hasPathInjection=97
137-
fm.createSymbolicLink(at: safeUrl, withDestinationURL: remoteUrl) // $ hasPathInjection=97
138-
fm.createSymbolicLink(atPath: remoteString, withDestinationPath: "") // $ hasPathInjection=97
139-
fm.createSymbolicLink(atPath: "", withDestinationPath: remoteString) // $ hasPathInjection=97
140-
fm.linkItem(at: remoteUrl, to: safeUrl) // $ hasPathInjection=97
141-
fm.linkItem(at: safeUrl, to: remoteUrl) // $ hasPathInjection=97
142-
fm.linkItem(atPath: remoteString, toPath: "") // $ hasPathInjection=97
143-
fm.linkItem(atPath: "", toPath: remoteString) // $ hasPathInjection=97
144-
let _ = fm.destinationOfSymbolicLink(atPath: remoteString) // $ hasPathInjection=97
145-
let _ = fm.fileExists(atPath: remoteString) // $ hasPathInjection=97
146-
let _ = fm.fileExists(atPath: remoteString, isDirectory: UnsafeMutablePointer<ObjCBool>.init(bitPattern: 0)) // $ hasPathInjection=97
147-
fm.setAttributes([:], ofItemAtPath: remoteString) // $ hasPathInjection=97
148-
let _ = fm.contents(atPath: remoteString) // $ hasPathInjection=97
149-
let _ = fm.contentsEqual(atPath: remoteString, andPath: "") // $ hasPathInjection=97
150-
let _ = fm.contentsEqual(atPath: "", andPath: remoteString) // $ hasPathInjection=97
151-
let _ = fm.changeCurrentDirectoryPath(remoteString) // $ hasPathInjection=97
152-
let _ = fm.unmountVolume(at: remoteUrl, options: [], completionHandler: { _ in }) // $ hasPathInjection=97
136+
let _ = fm.contentsOfDirectory(at: remoteUrl, includingPropertiesForKeys: [], options: []) // $ hasPathInjection=121
137+
let _ = fm.contentsOfDirectory(atPath: remoteString) // $ hasPathInjection=121
138+
let _ = fm.enumerator(at: remoteUrl, includingPropertiesForKeys: [], options: [], errorHandler: nil) // $ hasPathInjection=121
139+
let _ = fm.enumerator(atPath: remoteString) // $ hasPathInjection=121
140+
let _ = fm.subpathsOfDirectory(atPath: remoteString) // $ hasPathInjection=121
141+
let _ = fm.subpaths(atPath: remoteString) // $ hasPathInjection=121
142+
fm.createDirectory(at: remoteUrl, withIntermediateDirectories: false, attributes: [:]) // $ hasPathInjection=121
143+
let _ = fm.createDirectory(atPath: remoteString, attributes: [:]) // $ hasPathInjection=121
144+
let _ = fm.createFile(atPath: remoteString, contents: nil, attributes: [:]) // $ hasPathInjection=121
145+
fm.removeItem(at: remoteUrl) // $ hasPathInjection=121
146+
fm.removeItem(atPath: remoteString) // $ hasPathInjection=121
147+
fm.trashItem(at: remoteUrl, resultingItemURL: AutoreleasingUnsafeMutablePointer<NSURL?>()) // $ hasPathInjection=121
148+
let _ = fm.replaceItemAt(remoteUrl, withItemAt: safeUrl, backupItemName: nil, options: []) // $ hasPathInjection=121
149+
let _ = fm.replaceItemAt(safeUrl, withItemAt: remoteUrl, backupItemName: nil, options: []) // $ hasPathInjection=121
150+
fm.replaceItem(at: remoteUrl, withItemAt: safeUrl, backupItemName: nil, options: [], resultingItemURL: AutoreleasingUnsafeMutablePointer<NSURL?>()) // $ hasPathInjection=121
151+
fm.replaceItem(at: safeUrl, withItemAt: remoteUrl, backupItemName: nil, options: [], resultingItemURL: AutoreleasingUnsafeMutablePointer<NSURL?>()) // $ hasPathInjection=121
152+
fm.copyItem(at: remoteUrl, to: safeUrl) // $ hasPathInjection=121
153+
fm.copyItem(at: safeUrl, to: remoteUrl) // $ hasPathInjection=121
154+
fm.copyItem(atPath: remoteString, toPath: "") // $ hasPathInjection=121
155+
fm.copyItem(atPath: "", toPath: remoteString) // $ hasPathInjection=121
156+
fm.moveItem(at: remoteUrl, to: safeUrl) // $ hasPathInjection=121
157+
fm.moveItem(at: safeUrl, to: remoteUrl) // $ hasPathInjection=121
158+
fm.moveItem(atPath: remoteString, toPath: "") // $ hasPathInjection=121
159+
fm.moveItem(atPath: "", toPath: remoteString) // $ hasPathInjection=121
160+
fm.createSymbolicLink(at: remoteUrl, withDestinationURL: safeUrl) // $ hasPathInjection=121
161+
fm.createSymbolicLink(at: safeUrl, withDestinationURL: remoteUrl) // $ hasPathInjection=121
162+
fm.createSymbolicLink(atPath: remoteString, withDestinationPath: "") // $ hasPathInjection=121
163+
fm.createSymbolicLink(atPath: "", withDestinationPath: remoteString) // $ hasPathInjection=121
164+
fm.linkItem(at: remoteUrl, to: safeUrl) // $ hasPathInjection=121
165+
fm.linkItem(at: safeUrl, to: remoteUrl) // $ hasPathInjection=121
166+
fm.linkItem(atPath: remoteString, toPath: "") // $ hasPathInjection=121
167+
fm.linkItem(atPath: "", toPath: remoteString) // $ hasPathInjection=121
168+
let _ = fm.destinationOfSymbolicLink(atPath: remoteString) // $ hasPathInjection=121
169+
let _ = fm.fileExists(atPath: remoteString) // $ hasPathInjection=121
170+
let _ = fm.fileExists(atPath: remoteString, isDirectory: UnsafeMutablePointer<ObjCBool>.init(bitPattern: 0)) // $ hasPathInjection=121
171+
fm.setAttributes([:], ofItemAtPath: remoteString) // $ hasPathInjection=121
172+
let _ = fm.contents(atPath: remoteString) // $ hasPathInjection=121
173+
let _ = fm.contentsEqual(atPath: remoteString, andPath: "") // $ hasPathInjection=121
174+
let _ = fm.contentsEqual(atPath: "", andPath: remoteString) // $ hasPathInjection=121
175+
let _ = fm.changeCurrentDirectoryPath(remoteString) // $ hasPathInjection=121
176+
let _ = fm.unmountVolume(at: remoteUrl, options: [], completionHandler: { _ in }) // $ hasPathInjection=121
153177
// Deprecated methods
154-
let _ = fm.changeFileAttributes([:], atPath: remoteString) // $ hasPathInjection=97
155-
let _ = fm.directoryContents(atPath: remoteString) // $ hasPathInjection=97
156-
let _ = fm.createDirectory(atPath: remoteString, attributes: [:]) // $ hasPathInjection=97
157-
let _ = fm.createSymbolicLink(atPath: remoteString, pathContent: "") // $ hasPathInjection=97
158-
let _ = fm.createSymbolicLink(atPath: "", pathContent: remoteString) // $ hasPathInjection=97
159-
let _ = fm.pathContentOfSymbolicLink(atPath: remoteString) // $ hasPathInjection=97
160-
let _ = fm.replaceItemAtURL(originalItemURL: remoteNsUrl, withItemAtURL: safeNsUrl, backupItemName: nil, options: []) // $ hasPathInjection=97
161-
let _ = fm.replaceItemAtURL(originalItemURL: safeNsUrl, withItemAtURL: remoteNsUrl, backupItemName: nil, options: []) // $ hasPathInjection=97
178+
let _ = fm.changeFileAttributes([:], atPath: remoteString) // $ hasPathInjection=121
179+
let _ = fm.directoryContents(atPath: remoteString) // $ hasPathInjection=121
180+
let _ = fm.createDirectory(atPath: remoteString, attributes: [:]) // $ hasPathInjection=121
181+
let _ = fm.createSymbolicLink(atPath: remoteString, pathContent: "") // $ hasPathInjection=121
182+
let _ = fm.createSymbolicLink(atPath: "", pathContent: remoteString) // $ hasPathInjection=121
183+
let _ = fm.pathContentOfSymbolicLink(atPath: remoteString) // $ hasPathInjection=121
184+
let _ = fm.replaceItemAtURL(originalItemURL: remoteNsUrl, withItemAtURL: safeNsUrl, backupItemName: nil, options: []) // $ hasPathInjection=121
185+
let _ = fm.replaceItemAtURL(originalItemURL: safeNsUrl, withItemAtURL: remoteNsUrl, backupItemName: nil, options: []) // $ hasPathInjection=121
186+
187+
let _ = Database(path: remoteString, description: "", configuration: Configuration()) // $ hasPathInjection=121
188+
let _ = Database(path: "", description: "", configuration: Configuration()) // Safe
189+
let _ = DatabasePool(path: remoteString, configuration: Configuration()) // $ hasPathInjection=121
190+
let _ = DatabasePool(path: "", configuration: Configuration()) // Safe
191+
let _ = DatabaseQueue(path: remoteString, configuration: Configuration()) // $ hasPathInjection=121
192+
let _ = DatabaseQueue(path: "", configuration: Configuration()) // Safe
193+
let _ = DatabaseSnapshotPool(path: remoteString, configuration: Configuration()) // $ hasPathInjection=121
194+
let _ = DatabaseSnapshotPool(path: "", configuration: Configuration()) // Safe
195+
let _ = SerializedDatabase(path: remoteString, defaultLabel: "") // $ hasPathInjection=121
196+
let _ = SerializedDatabase(path: "", defaultLabel: "") // Safe
197+
let _ = SerializedDatabase(path: remoteString, defaultLabel: "", purpose: nil) // $ hasPathInjection=121
198+
let _ = SerializedDatabase(path: "", defaultLabel: "", purpose: nil) // Safe
199+
let _ = SerializedDatabase(path: remoteString, configuration: Configuration(), defaultLabel: "") // $ hasPathInjection=121
200+
let _ = SerializedDatabase(path: "", configuration: Configuration(), defaultLabel: "") // Safe
201+
let _ = SerializedDatabase(path: remoteString, configuration: Configuration(), defaultLabel: "", purpose: nil) // $ hasPathInjection=121
202+
let _ = SerializedDatabase(path: "", configuration: Configuration(), defaultLabel: "", purpose: nil) // Safe
162203
}
163204

164205
func testSanitizers() {
@@ -170,5 +211,5 @@ func testSanitizers() {
170211
if (filePath.lexicallyNormalized().starts(with: FilePath(stringLiteral: "/safe"))) {
171212
fm.contents(atPath: remoteString) // Safe
172213
}
173-
fm.contents(atPath: remoteString) // $ hasPathInjection=165
214+
fm.contents(atPath: remoteString) // $ hasPathInjection=206
174215
}

0 commit comments

Comments
 (0)