Skip to content

Commit c6d9e1e

Browse files
atorralbaatorralba
authored
Merge pull request #11742 from atorralba/atorralba/swift/more-path-injection-sinks
Swift: Add more path injection sinks
2 parents 5d54482 + c115a9f commit c6d9e1e

File tree

2 files changed

+129
-68
lines changed

2 files changed

+129
-68
lines changed

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,15 @@ private class PathInjectionSinks extends SinkModelCsv {
105105
";NIOFileHandle;true;init(descriptor:);;;Argument[0];path-injection",
106106
";NIOFileHandle;true;init(path:mode:flags:);;;Argument[0];path-injection",
107107
";NIOFileHandle;true;init(path:);;;Argument[0];path-injection",
108+
";NSString;true;write(to:atomically:encoding:);;;Argument[0];path-injection",
109+
";NSString;true;write(toFile:atomically:encoding:);;;Argument[0];path-injection",
110+
";NSKeyedUnarchiver;true;unarchiveObject(withFile:);;;Argument[0];path-injection",
111+
";ArchiveByteStream;true;fileStream(fd:automaticClose:);;;Argument[0];path-injection",
112+
";ArchiveByteStream;true;withFileStream(fd:automaticClose:_:);;;Argument[0];path-injection",
113+
";ArchiveByteStream;true;fileStream(path:mode:options:permissions:);;;Argument[0];path-injection",
114+
";ArchiveByteStream;true;withFileStream(path:mode:options:permissions:_:);;;Argument[0];path-injection",
115+
";Bundle;true;init(url:);;;Argument[0];path-injection",
116+
";Bundle;true;init(path:);;;Argument[0];path-injection",
108117
// GRDB
109118
";Database;true;init(path:description:configuration:);;;Argument[0];path-injection",
110119
";DatabasePool;true;init(path:configuration:);;;Argument[0];path-injection",

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

Lines changed: 120 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@ extension String {
1515
}
1616
}
1717

18+
class NSString {
19+
func write(toFile: String, atomically: Bool, encoding: UInt) {}
20+
func write(to: URL, atomically: Bool, encoding: UInt) {}
21+
}
22+
1823
class Data {
1924
struct WritingOptions : OptionSet { let rawValue: Int }
2025
init<S>(_ elements: S) {}
@@ -29,6 +34,10 @@ class NSData {
2934
func write(toFile: String, options: NSData.WritingOptions) {}
3035
}
3136

37+
class NSKeyedUnarchiver {
38+
func unarchiveObject(withFile: String) -> Any? { return nil }
39+
}
40+
3241
struct URLResourceKey {}
3342

3443
struct FileAttributeKey : Hashable {}
@@ -37,9 +46,10 @@ struct ObjCBool {}
3746

3847
struct AutoreleasingUnsafeMutablePointer<Pointee> {}
3948

40-
struct FilePath {
49+
struct FilePath : ExpressibleByStringLiteral {
50+
typealias StringLiteralType = String
4151
init(stringLiteral: String) {}
42-
func lexicallyNormalized() -> FilePath { return FilePath(stringLiteral: "") }
52+
func lexicallyNormalized() -> FilePath { return "" }
4353
func starts(with: FilePath) -> Bool { return false }
4454
}
4555

@@ -91,6 +101,38 @@ class FileManager {
91101
func replaceItemAtURL(originalItemURL: NSURL, withItemAtURL: NSURL, backupItemName: String?, options: FileManager.ItemReplacementOptions) -> NSURL? { return nil }
92102
}
93103

104+
struct FileDescriptor {
105+
struct AccessMode : RawRepresentable {
106+
static let readOnly = AccessMode(rawValue: 0)
107+
let rawValue: UInt8
108+
init(rawValue: UInt8) { self.rawValue = rawValue}
109+
}
110+
111+
struct OpenOptions : RawRepresentable {
112+
static let append = OpenOptions(rawValue: 0)
113+
let rawValue: UInt8
114+
init(rawValue: UInt8) { self.rawValue = rawValue}
115+
}
116+
}
117+
118+
struct FilePermissions : RawRepresentable {
119+
static let ownerRead = FilePermissions(rawValue: 0)
120+
let rawValue: UInt8
121+
init(rawValue: UInt8) { self.rawValue = rawValue}
122+
}
123+
124+
class ArchiveByteStream {
125+
static func fileStream(fd: FileDescriptor, automaticClose: Bool = true) -> ArchiveByteStream? { return nil }
126+
static func withFileStream<E>(fd: FileDescriptor, automaticClose: Bool = true, _ body: (ArchiveByteStream) -> E) -> E { return body(ArchiveByteStream()) }
127+
static func fileStream(path: FilePath, mode: FileDescriptor.AccessMode, options: FileDescriptor.OpenOptions, permissions: FilePermissions) -> ArchiveByteStream? { return nil }
128+
static func withFileStream<E>(path: FilePath, mode: FileDescriptor.AccessMode, options: FileDescriptor.OpenOptions, permissions: FilePermissions, _ body: (ArchiveByteStream) -> E) -> E { return body(ArchiveByteStream()) }
129+
}
130+
131+
class Bundle {
132+
init?(url: URL) {}
133+
init?(path: String) {}
134+
}
135+
94136
// GRDB
95137

96138
struct Configuration {}
@@ -124,81 +166,91 @@ func test() {
124166
let safeUrl = URL(string: "")!
125167
let safeNsUrl = NSURL(string: "")!
126168

127-
Data("").write(to: remoteUrl, options: []) // $ hasPathInjection=121
169+
Data("").write(to: remoteUrl, options: []) // $ hasPathInjection=163
128170

129171
let nsData = NSData()
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
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
134176

135177
let fm = FileManager()
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
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
177219
// Deprecated methods
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
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
188240
let _ = Database(path: "", description: "", configuration: Configuration()) // Safe
189-
let _ = DatabasePool(path: remoteString, configuration: Configuration()) // $ hasPathInjection=121
241+
let _ = DatabasePool(path: remoteString, configuration: Configuration()) // $ hasPathInjection=163
190242
let _ = DatabasePool(path: "", configuration: Configuration()) // Safe
191-
let _ = DatabaseQueue(path: remoteString, configuration: Configuration()) // $ hasPathInjection=121
243+
let _ = DatabaseQueue(path: remoteString, configuration: Configuration()) // $ hasPathInjection=163
192244
let _ = DatabaseQueue(path: "", configuration: Configuration()) // Safe
193-
let _ = DatabaseSnapshotPool(path: remoteString, configuration: Configuration()) // $ hasPathInjection=121
245+
let _ = DatabaseSnapshotPool(path: remoteString, configuration: Configuration()) // $ hasPathInjection=163
194246
let _ = DatabaseSnapshotPool(path: "", configuration: Configuration()) // Safe
195-
let _ = SerializedDatabase(path: remoteString, defaultLabel: "") // $ hasPathInjection=121
247+
let _ = SerializedDatabase(path: remoteString, defaultLabel: "") // $ hasPathInjection=163
196248
let _ = SerializedDatabase(path: "", defaultLabel: "") // Safe
197-
let _ = SerializedDatabase(path: remoteString, defaultLabel: "", purpose: nil) // $ hasPathInjection=121
249+
let _ = SerializedDatabase(path: remoteString, defaultLabel: "", purpose: nil) // $ hasPathInjection=163
198250
let _ = SerializedDatabase(path: "", defaultLabel: "", purpose: nil) // Safe
199-
let _ = SerializedDatabase(path: remoteString, configuration: Configuration(), defaultLabel: "") // $ hasPathInjection=121
251+
let _ = SerializedDatabase(path: remoteString, configuration: Configuration(), defaultLabel: "") // $ hasPathInjection=163
200252
let _ = SerializedDatabase(path: "", configuration: Configuration(), defaultLabel: "") // Safe
201-
let _ = SerializedDatabase(path: remoteString, configuration: Configuration(), defaultLabel: "", purpose: nil) // $ hasPathInjection=121
253+
let _ = SerializedDatabase(path: remoteString, configuration: Configuration(), defaultLabel: "", purpose: nil) // $ hasPathInjection=163
202254
let _ = SerializedDatabase(path: "", configuration: Configuration(), defaultLabel: "", purpose: nil) // Safe
203255
}
204256

@@ -208,8 +260,8 @@ func testSanitizers() {
208260
let fm = FileManager()
209261

210262
let filePath = FilePath(stringLiteral: remoteString)
211-
if (filePath.lexicallyNormalized().starts(with: FilePath(stringLiteral: "/safe"))) {
212-
fm.contents(atPath: remoteString) // Safe
263+
if (filePath.lexicallyNormalized().starts(with: "/safe")) {
264+
let _ = fm.contents(atPath: remoteString) // Safe
213265
}
214-
fm.contents(atPath: remoteString) // $ hasPathInjection=206
266+
let _ = fm.contents(atPath: remoteString) // $ hasPathInjection=258
215267
}

0 commit comments

Comments
 (0)