Skip to content

Commit cb11524

Browse files
authored
Merge pull request github#12154 from geoffw0/pathinjectionext
Swift: More path injection sinks
2 parents 2b529fb + ad85b37 commit cb11524

File tree

2 files changed

+107
-75
lines changed

2 files changed

+107
-75
lines changed

swift/ql/lib/codeql/swift/security/PathInjectionExtensions.qll

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,11 @@ private class PathInjectionSinks extends SinkModelCsv {
108108
";NIOFileHandle;true;init(descriptor:);;;Argument[0];path-injection",
109109
";NIOFileHandle;true;init(path:mode:flags:);;;Argument[0];path-injection",
110110
";NIOFileHandle;true;init(path:);;;Argument[0];path-injection",
111+
";String;true;init(contentsOfFile:);;;Argument[0];path-injection",
112+
";String;true;init(contentsOfFile:encoding:);;;Argument[0];path-injection",
113+
";String;true;init(contentsOfFile:usedEncoding:);;;Argument[0];path-injection",
114+
";NSString;true;init(contentsOfFile:encoding:);;;Argument[0];path-injection",
115+
";NSString;true;init(contentsOfFile:usedEncoding:);;;Argument[0];path-injection",
111116
";NSString;true;write(to:atomically:encoding:);;;Argument[0];path-injection",
112117
";NSString;true;write(toFile:atomically:encoding:);;;Argument[0];path-injection",
113118
";NSKeyedUnarchiver;true;unarchiveObject(withFile:);;;Argument[0];path-injection",

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

Lines changed: 102 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,32 @@ class NSURL {
99
}
1010

1111
extension String {
12-
init(contentsOf: URL) {
12+
struct Encoding {
13+
static let utf8 = Encoding()
14+
}
15+
16+
init(contentsOf: URL) {
1317
let data = ""
1418
self.init(data)
1519
}
20+
21+
init(contentsOfFile path: String) throws {
22+
self.init("")
23+
}
24+
25+
init(contentsOfFile path: String, encoding enc: String.Encoding) throws {
26+
self.init("")
27+
}
28+
29+
init(contentsOfFile path: String, usedEncoding: inout String.Encoding) throws {
30+
self.init("")
31+
}
1632
}
1733

1834
class NSString {
35+
convenience init(contentsOfFile path: String, encoding enc: UInt) throws { self.init() }
36+
convenience init(contentsOfFile path: String, usedEncoding enc: UnsafeMutablePointer<UInt>?) throws { self.init() }
37+
1938
func write(toFile: String, atomically: Bool, encoding: UInt) {}
2039
func write(to: URL, atomically: Bool, encoding: UInt) {}
2140
}
@@ -166,91 +185,99 @@ func test() {
166185
let safeUrl = URL(string: "")!
167186
let safeNsUrl = NSURL(string: "")!
168187

169-
Data("").write(to: remoteUrl, options: []) // $ hasPathInjection=163
188+
Data("").write(to: remoteUrl, options: []) // $ hasPathInjection=182
170189

171190
let nsData = NSData()
172-
let _ = nsData.write(to: remoteUrl, atomically: false) // $ hasPathInjection=163
173-
nsData.write(to: remoteUrl, options: []) // $ hasPathInjection=163
174-
let _ = nsData.write(toFile: remoteString, atomically: false) // $ hasPathInjection=163
175-
nsData.write(toFile: remoteString, options: []) // $ hasPathInjection=163
191+
let _ = nsData.write(to: remoteUrl, atomically: false) // $ hasPathInjection=182
192+
nsData.write(to: remoteUrl, options: []) // $ hasPathInjection=182
193+
let _ = nsData.write(toFile: remoteString, atomically: false) // $ hasPathInjection=182
194+
nsData.write(toFile: remoteString, options: []) // $ hasPathInjection=182
176195

177196
let fm = FileManager()
178-
let _ = fm.contentsOfDirectory(at: remoteUrl, includingPropertiesForKeys: [], options: []) // $ hasPathInjection=163
179-
let _ = fm.contentsOfDirectory(atPath: remoteString) // $ hasPathInjection=163
180-
let _ = fm.enumerator(at: remoteUrl, includingPropertiesForKeys: [], options: [], errorHandler: nil) // $ hasPathInjection=163
181-
let _ = fm.enumerator(atPath: remoteString) // $ hasPathInjection=163
182-
let _ = fm.subpathsOfDirectory(atPath: remoteString) // $ hasPathInjection=163
183-
let _ = fm.subpaths(atPath: remoteString) // $ hasPathInjection=163
184-
fm.createDirectory(at: remoteUrl, withIntermediateDirectories: false, attributes: [:]) // $ hasPathInjection=163
185-
let _ = fm.createDirectory(atPath: remoteString, attributes: [:]) // $ hasPathInjection=163
186-
let _ = fm.createFile(atPath: remoteString, contents: nil, attributes: [:]) // $ hasPathInjection=163
187-
fm.removeItem(at: remoteUrl) // $ hasPathInjection=163
188-
fm.removeItem(atPath: remoteString) // $ hasPathInjection=163
189-
fm.trashItem(at: remoteUrl, resultingItemURL: AutoreleasingUnsafeMutablePointer<NSURL?>()) // $ hasPathInjection=163
190-
let _ = fm.replaceItemAt(remoteUrl, withItemAt: safeUrl, backupItemName: nil, options: []) // $ hasPathInjection=163
191-
let _ = fm.replaceItemAt(safeUrl, withItemAt: remoteUrl, backupItemName: nil, options: []) // $ hasPathInjection=163
192-
fm.replaceItem(at: remoteUrl, withItemAt: safeUrl, backupItemName: nil, options: [], resultingItemURL: AutoreleasingUnsafeMutablePointer<NSURL?>()) // $ hasPathInjection=163
193-
fm.replaceItem(at: safeUrl, withItemAt: remoteUrl, backupItemName: nil, options: [], resultingItemURL: AutoreleasingUnsafeMutablePointer<NSURL?>()) // $ hasPathInjection=163
194-
fm.copyItem(at: remoteUrl, to: safeUrl) // $ hasPathInjection=163
195-
fm.copyItem(at: safeUrl, to: remoteUrl) // $ hasPathInjection=163
196-
fm.copyItem(atPath: remoteString, toPath: "") // $ hasPathInjection=163
197-
fm.copyItem(atPath: "", toPath: remoteString) // $ hasPathInjection=163
198-
fm.moveItem(at: remoteUrl, to: safeUrl) // $ hasPathInjection=163
199-
fm.moveItem(at: safeUrl, to: remoteUrl) // $ hasPathInjection=163
200-
fm.moveItem(atPath: remoteString, toPath: "") // $ hasPathInjection=163
201-
fm.moveItem(atPath: "", toPath: remoteString) // $ hasPathInjection=163
202-
fm.createSymbolicLink(at: remoteUrl, withDestinationURL: safeUrl) // $ hasPathInjection=163
203-
fm.createSymbolicLink(at: safeUrl, withDestinationURL: remoteUrl) // $ hasPathInjection=163
204-
fm.createSymbolicLink(atPath: remoteString, withDestinationPath: "") // $ hasPathInjection=163
205-
fm.createSymbolicLink(atPath: "", withDestinationPath: remoteString) // $ hasPathInjection=163
206-
fm.linkItem(at: remoteUrl, to: safeUrl) // $ hasPathInjection=163
207-
fm.linkItem(at: safeUrl, to: remoteUrl) // $ hasPathInjection=163
208-
fm.linkItem(atPath: remoteString, toPath: "") // $ hasPathInjection=163
209-
fm.linkItem(atPath: "", toPath: remoteString) // $ hasPathInjection=163
210-
let _ = fm.destinationOfSymbolicLink(atPath: remoteString) // $ hasPathInjection=163
211-
let _ = fm.fileExists(atPath: remoteString) // $ hasPathInjection=163
212-
let _ = fm.fileExists(atPath: remoteString, isDirectory: UnsafeMutablePointer<ObjCBool>.init(bitPattern: 0)) // $ hasPathInjection=163
213-
fm.setAttributes([:], ofItemAtPath: remoteString) // $ hasPathInjection=163
214-
let _ = fm.contents(atPath: remoteString) // $ hasPathInjection=163
215-
let _ = fm.contentsEqual(atPath: remoteString, andPath: "") // $ hasPathInjection=163
216-
let _ = fm.contentsEqual(atPath: "", andPath: remoteString) // $ hasPathInjection=163
217-
let _ = fm.changeCurrentDirectoryPath(remoteString) // $ hasPathInjection=163
218-
let _ = fm.unmountVolume(at: remoteUrl, options: [], completionHandler: { _ in }) // $ hasPathInjection=163
197+
let _ = fm.contentsOfDirectory(at: remoteUrl, includingPropertiesForKeys: [], options: []) // $ hasPathInjection=182
198+
let _ = fm.contentsOfDirectory(atPath: remoteString) // $ hasPathInjection=182
199+
let _ = fm.enumerator(at: remoteUrl, includingPropertiesForKeys: [], options: [], errorHandler: nil) // $ hasPathInjection=182
200+
let _ = fm.enumerator(atPath: remoteString) // $ hasPathInjection=182
201+
let _ = fm.subpathsOfDirectory(atPath: remoteString) // $ hasPathInjection=182
202+
let _ = fm.subpaths(atPath: remoteString) // $ hasPathInjection=182
203+
fm.createDirectory(at: remoteUrl, withIntermediateDirectories: false, attributes: [:]) // $ hasPathInjection=182
204+
let _ = fm.createDirectory(atPath: remoteString, attributes: [:]) // $ hasPathInjection=182
205+
let _ = fm.createFile(atPath: remoteString, contents: nil, attributes: [:]) // $ hasPathInjection=182
206+
fm.removeItem(at: remoteUrl) // $ hasPathInjection=182
207+
fm.removeItem(atPath: remoteString) // $ hasPathInjection=182
208+
fm.trashItem(at: remoteUrl, resultingItemURL: AutoreleasingUnsafeMutablePointer<NSURL?>()) // $ hasPathInjection=182
209+
let _ = fm.replaceItemAt(remoteUrl, withItemAt: safeUrl, backupItemName: nil, options: []) // $ hasPathInjection=182
210+
let _ = fm.replaceItemAt(safeUrl, withItemAt: remoteUrl, backupItemName: nil, options: []) // $ hasPathInjection=182
211+
fm.replaceItem(at: remoteUrl, withItemAt: safeUrl, backupItemName: nil, options: [], resultingItemURL: AutoreleasingUnsafeMutablePointer<NSURL?>()) // $ hasPathInjection=182
212+
fm.replaceItem(at: safeUrl, withItemAt: remoteUrl, backupItemName: nil, options: [], resultingItemURL: AutoreleasingUnsafeMutablePointer<NSURL?>()) // $ hasPathInjection=182
213+
fm.copyItem(at: remoteUrl, to: safeUrl) // $ hasPathInjection=182
214+
fm.copyItem(at: safeUrl, to: remoteUrl) // $ hasPathInjection=182
215+
fm.copyItem(atPath: remoteString, toPath: "") // $ hasPathInjection=182
216+
fm.copyItem(atPath: "", toPath: remoteString) // $ hasPathInjection=182
217+
fm.moveItem(at: remoteUrl, to: safeUrl) // $ hasPathInjection=182
218+
fm.moveItem(at: safeUrl, to: remoteUrl) // $ hasPathInjection=182
219+
fm.moveItem(atPath: remoteString, toPath: "") // $ hasPathInjection=182
220+
fm.moveItem(atPath: "", toPath: remoteString) // $ hasPathInjection=182
221+
fm.createSymbolicLink(at: remoteUrl, withDestinationURL: safeUrl) // $ hasPathInjection=182
222+
fm.createSymbolicLink(at: safeUrl, withDestinationURL: remoteUrl) // $ hasPathInjection=182
223+
fm.createSymbolicLink(atPath: remoteString, withDestinationPath: "") // $ hasPathInjection=182
224+
fm.createSymbolicLink(atPath: "", withDestinationPath: remoteString) // $ hasPathInjection=182
225+
fm.linkItem(at: remoteUrl, to: safeUrl) // $ hasPathInjection=182
226+
fm.linkItem(at: safeUrl, to: remoteUrl) // $ hasPathInjection=182
227+
fm.linkItem(atPath: remoteString, toPath: "") // $ hasPathInjection=182
228+
fm.linkItem(atPath: "", toPath: remoteString) // $ hasPathInjection=182
229+
let _ = fm.destinationOfSymbolicLink(atPath: remoteString) // $ hasPathInjection=182
230+
let _ = fm.fileExists(atPath: remoteString) // $ hasPathInjection=182
231+
let _ = fm.fileExists(atPath: remoteString, isDirectory: UnsafeMutablePointer<ObjCBool>.init(bitPattern: 0)) // $ hasPathInjection=182
232+
fm.setAttributes([:], ofItemAtPath: remoteString) // $ hasPathInjection=182
233+
let _ = fm.contents(atPath: remoteString) // $ hasPathInjection=182
234+
let _ = fm.contentsEqual(atPath: remoteString, andPath: "") // $ hasPathInjection=182
235+
let _ = fm.contentsEqual(atPath: "", andPath: remoteString) // $ hasPathInjection=182
236+
let _ = fm.changeCurrentDirectoryPath(remoteString) // $ hasPathInjection=182
237+
let _ = fm.unmountVolume(at: remoteUrl, options: [], completionHandler: { _ in }) // $ hasPathInjection=182
219238
// Deprecated methods
220-
let _ = fm.changeFileAttributes([:], atPath: remoteString) // $ hasPathInjection=163
221-
let _ = fm.directoryContents(atPath: remoteString) // $ hasPathInjection=163
222-
let _ = fm.createDirectory(atPath: remoteString, attributes: [:]) // $ hasPathInjection=163
223-
let _ = fm.createSymbolicLink(atPath: remoteString, pathContent: "") // $ hasPathInjection=163
224-
let _ = fm.createSymbolicLink(atPath: "", pathContent: remoteString) // $ hasPathInjection=163
225-
let _ = fm.pathContentOfSymbolicLink(atPath: remoteString) // $ hasPathInjection=163
226-
let _ = fm.replaceItemAtURL(originalItemURL: remoteNsUrl, withItemAtURL: safeNsUrl, backupItemName: nil, options: []) // $ hasPathInjection=163
227-
let _ = fm.replaceItemAtURL(originalItemURL: safeNsUrl, withItemAtURL: remoteNsUrl, backupItemName: nil, options: []) // $ hasPathInjection=163
228-
229-
NSString().write(to: remoteUrl, atomically: true, encoding: 0) // $ hasPathInjection=163
230-
NSString().write(toFile: remoteString, atomically: true, encoding: 0) // $ hasPathInjection=163
231-
let _ = NSKeyedUnarchiver().unarchiveObject(withFile: remoteString) // $ hasPathInjection=163
232-
let _ = ArchiveByteStream.fileStream(fd: remoteString as! FileDescriptor, automaticClose: true) // $ hasPathInjection=163
233-
ArchiveByteStream.withFileStream(fd: remoteString as! FileDescriptor, automaticClose: true) { _ in } // $ hasPathInjection=163
234-
let _ = ArchiveByteStream.fileStream(path: FilePath(stringLiteral: remoteString), mode: .readOnly, options: .append, permissions: .ownerRead) // $ hasPathInjection=163
235-
ArchiveByteStream.withFileStream(path: FilePath(stringLiteral: remoteString), mode: .readOnly, options: .append, permissions: .ownerRead) { _ in } // $ hasPathInjection=163
236-
let _ = Bundle(url: remoteUrl) // $ hasPathInjection=163
237-
let _ = Bundle(path: remoteString) // $ hasPathInjection=163
238-
239-
let _ = Database(path: remoteString, description: "", configuration: Configuration()) // $ hasPathInjection=163
239+
let _ = fm.changeFileAttributes([:], atPath: remoteString) // $ hasPathInjection=182
240+
let _ = fm.directoryContents(atPath: remoteString) // $ hasPathInjection=182
241+
let _ = fm.createDirectory(atPath: remoteString, attributes: [:]) // $ hasPathInjection=182
242+
let _ = fm.createSymbolicLink(atPath: remoteString, pathContent: "") // $ hasPathInjection=182
243+
let _ = fm.createSymbolicLink(atPath: "", pathContent: remoteString) // $ hasPathInjection=182
244+
let _ = fm.pathContentOfSymbolicLink(atPath: remoteString) // $ hasPathInjection=182
245+
let _ = fm.replaceItemAtURL(originalItemURL: remoteNsUrl, withItemAtURL: safeNsUrl, backupItemName: nil, options: []) // $ hasPathInjection=182
246+
let _ = fm.replaceItemAtURL(originalItemURL: safeNsUrl, withItemAtURL: remoteNsUrl, backupItemName: nil, options: []) // $ hasPathInjection=182
247+
248+
var encoding = String.Encoding.utf8
249+
let _ = try! String(contentsOfFile: remoteString) // $ hasPathInjection=182
250+
let _ = try! String(contentsOfFile: remoteString, encoding: String.Encoding.utf8) // $ hasPathInjection=182
251+
let _ = try! String(contentsOfFile: remoteString, usedEncoding: &encoding) // $ hasPathInjection=182
252+
253+
let _ = try! NSString(contentsOfFile: remoteString, encoding: 0) // $ hasPathInjection=182
254+
let _ = try! NSString(contentsOfFile: remoteString, usedEncoding: nil) // $ hasPathInjection=182
255+
NSString().write(to: remoteUrl, atomically: true, encoding: 0) // $ hasPathInjection=182
256+
NSString().write(toFile: remoteString, atomically: true, encoding: 0) // $ hasPathInjection=182
257+
258+
let _ = NSKeyedUnarchiver().unarchiveObject(withFile: remoteString) // $ hasPathInjection=182
259+
let _ = ArchiveByteStream.fileStream(fd: remoteString as! FileDescriptor, automaticClose: true) // $ hasPathInjection=182
260+
ArchiveByteStream.withFileStream(fd: remoteString as! FileDescriptor, automaticClose: true) { _ in } // $ hasPathInjection=182
261+
let _ = ArchiveByteStream.fileStream(path: FilePath(stringLiteral: remoteString), mode: .readOnly, options: .append, permissions: .ownerRead) // $ hasPathInjection=182
262+
ArchiveByteStream.withFileStream(path: FilePath(stringLiteral: remoteString), mode: .readOnly, options: .append, permissions: .ownerRead) { _ in } // $ hasPathInjection=182
263+
let _ = Bundle(url: remoteUrl) // $ hasPathInjection=182
264+
let _ = Bundle(path: remoteString) // $ hasPathInjection=182
265+
266+
let _ = Database(path: remoteString, description: "", configuration: Configuration()) // $ hasPathInjection=182
240267
let _ = Database(path: "", description: "", configuration: Configuration()) // Safe
241-
let _ = DatabasePool(path: remoteString, configuration: Configuration()) // $ hasPathInjection=163
268+
let _ = DatabasePool(path: remoteString, configuration: Configuration()) // $ hasPathInjection=182
242269
let _ = DatabasePool(path: "", configuration: Configuration()) // Safe
243-
let _ = DatabaseQueue(path: remoteString, configuration: Configuration()) // $ hasPathInjection=163
270+
let _ = DatabaseQueue(path: remoteString, configuration: Configuration()) // $ hasPathInjection=182
244271
let _ = DatabaseQueue(path: "", configuration: Configuration()) // Safe
245-
let _ = DatabaseSnapshotPool(path: remoteString, configuration: Configuration()) // $ hasPathInjection=163
272+
let _ = DatabaseSnapshotPool(path: remoteString, configuration: Configuration()) // $ hasPathInjection=182
246273
let _ = DatabaseSnapshotPool(path: "", configuration: Configuration()) // Safe
247-
let _ = SerializedDatabase(path: remoteString, defaultLabel: "") // $ hasPathInjection=163
274+
let _ = SerializedDatabase(path: remoteString, defaultLabel: "") // $ hasPathInjection=182
248275
let _ = SerializedDatabase(path: "", defaultLabel: "") // Safe
249-
let _ = SerializedDatabase(path: remoteString, defaultLabel: "", purpose: nil) // $ hasPathInjection=163
276+
let _ = SerializedDatabase(path: remoteString, defaultLabel: "", purpose: nil) // $ hasPathInjection=182
250277
let _ = SerializedDatabase(path: "", defaultLabel: "", purpose: nil) // Safe
251-
let _ = SerializedDatabase(path: remoteString, configuration: Configuration(), defaultLabel: "") // $ hasPathInjection=163
278+
let _ = SerializedDatabase(path: remoteString, configuration: Configuration(), defaultLabel: "") // $ hasPathInjection=182
252279
let _ = SerializedDatabase(path: "", configuration: Configuration(), defaultLabel: "") // Safe
253-
let _ = SerializedDatabase(path: remoteString, configuration: Configuration(), defaultLabel: "", purpose: nil) // $ hasPathInjection=163
280+
let _ = SerializedDatabase(path: remoteString, configuration: Configuration(), defaultLabel: "", purpose: nil) // $ hasPathInjection=182
254281
let _ = SerializedDatabase(path: "", configuration: Configuration(), defaultLabel: "", purpose: nil) // Safe
255282
}
256283

@@ -263,5 +290,5 @@ func testSanitizers() {
263290
if (filePath.lexicallyNormalized().starts(with: "/safe")) {
264291
let _ = fm.contents(atPath: remoteString) // Safe
265292
}
266-
let _ = fm.contents(atPath: remoteString) // $ hasPathInjection=258
293+
let _ = fm.contents(atPath: remoteString) // $ hasPathInjection=285
267294
}

0 commit comments

Comments
 (0)