Skip to content

Commit e222807

Browse files
atorralbaatorralba
committed
Remove dubious sinks
1 parent 56f5214 commit e222807

File tree

2 files changed

+55
-93
lines changed

2 files changed

+55
-93
lines changed

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

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -90,25 +90,13 @@ private class PathInjectionSinks extends SinkModelCsv {
9090
";FileManager;true;destinationOfSymbolicLink(atPath:);;;Argument[0];path-injection",
9191
";FileManager;true;fileExists(atPath:);;;Argument[0];path-injection",
9292
";FileManager;true;fileExists(atPath:isDirectory:);;;Argument[0];path-injection",
93-
";FileManager;true;isReadableFile(atPath:);;;Argument[0];path-injection",
94-
";FileManager;true;isWritableFile(atPath:);;;Argument[0];path-injection",
95-
";FileManager;true;isDeletableFile(atPath:);;;Argument[0];path-injection",
96-
";FileManager;true;componentsToDisplay(forPath:);;;Argument[0];path-injection",
97-
";FileManager;true;displayName(atPath:);;;Argument[0];path-injection",
98-
";FileManager;true;attributesOfItem(atPath:);;;Argument[0];path-injection",
99-
";FileManager;true;attributesOfFileSystem(forPath:);;;Argument[0];path-injection",
10093
";FileManager;true;setAttributes(_:ofItemAtPath:);;;Argument[1];path-injection",
10194
";FileManager;true;contents(atPath:);;;Argument[0];path-injection",
10295
";FileManager;true;contentsEqual(atPath:andPath:);;;Argument[0..1];path-injection",
103-
";FileManager;true;getRelationship(_:ofDirectoryAt:toItemAt:);;;Argument[1..2];path-injection",
104-
";FileManager;true;getRelationship(_:of:in:toItemAt:);;;Argument[3];path-injection",
10596
";FileManager;true;changeCurrentDirectoryPath(_:);;;Argument[0];path-injection",
10697
";FileManager;true;unmountVolume(at:options:completionHandler:);;;Argument[0];path-injection",
107-
";FileManager;true;NSHFSTypeOfFile(_:);;;Argument[0];path-injection",
10898
// Deprecated FileManager methods:
10999
";FileManager;true;changeFileAttributes(_:atPath:);;;Argument[1];path-injection",
110-
";FileManager;true;fileAttributes(atPath:traverseLink:);;;Argument[0];path-injection",
111-
";FileManager;true;fileSystemAttributes(atPath:);;;Argument[0];path-injection",
112100
";FileManager;true;directoryContents(atPath:);;;Argument[0];path-injection",
113101
";FileManager;true;createDirectory(atPath:attributes:);;;Argument[0];path-injection",
114102
";FileManager;true;createSymbolicLink(atPath:pathContent:);;;Argument[0..1];path-injection",

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

Lines changed: 55 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -77,26 +77,13 @@ class FileManager {
7777
func destinationOfSymbolicLink(atPath: String) -> String { return "" }
7878
func fileExists(atPath: String) -> Bool { return false }
7979
func fileExists(atPath: String, isDirectory: UnsafeMutablePointer<ObjCBool>?) -> Bool { return false }
80-
func isReadableFile(atPath: String) -> Bool { return false }
81-
func isWritableFile(atPath: String) -> Bool { return false }
82-
func isExecutableFile(atPath: String) -> Bool { return false }
83-
func isDeletableFile(atPath: String) -> Bool { return false }
84-
func componentsToDisplay(forPath: String) -> [String]? { return nil }
85-
func displayName(atPath: String) -> String { return "" }
86-
func attributesOfItem(atPath: String) -> [FileAttributeKey : Any] { return [:] }
87-
func attributesOfFileSystem(forPath: String) -> [FileAttributeKey : Any] { return [:] }
8880
func setAttributes(_: [FileAttributeKey : Any], ofItemAtPath: String) {}
8981
func contents(atPath: String) -> Data? { return nil }
9082
func contentsEqual(atPath: String, andPath: String) -> Bool { return false }
91-
func getRelationship(_: UnsafeMutablePointer<FileManager.URLRelationship>, ofDirectoryAt: URL, toItemAt: URL) {}
92-
func getRelationship(_: UnsafeMutablePointer<FileManager.URLRelationship>, of: FileManager.SearchPathDirectory, in: FileManager.SearchPathDomainMask, toItemAt: URL) {}
9383
func changeCurrentDirectoryPath(_: String) -> Bool { return false }
9484
func unmountVolume(at: URL, options: FileManager.UnmountOptions, completionHandler: (Error?) -> Void) {}
95-
func NSHFSTypeOfFile(_: String!) -> String! { return "" }
9685
// Deprecated methods
9786
func changeFileAttributes(_: [AnyHashable : Any], atPath: String) -> Bool { return false }
98-
func fileAttributes(atPath: String, traverseLink: Bool) -> [AnyHashable : Any]? { return nil }
99-
func fileSystemAttributes(atPath: String) -> [AnyHashable : Any]? { return nil }
10087
func directoryContents(atPath: String) -> [Any]? { return nil }
10188
func createDirectory(atPath: String, attributes: [AnyHashable : Any]) -> Bool { return false }
10289
func createSymbolicLink(atPath: String, pathContent: String) -> Bool { return false }
@@ -113,78 +100,65 @@ func test() {
113100
let safeUrl = URL(string: "")!
114101
let safeNsUrl = NSURL(string: "")!
115102

116-
Data("").write(to: remoteUrl, options: []) // $ hasPathInjection=110
103+
Data("").write(to: remoteUrl, options: []) // $ hasPathInjection=97
117104

118105
let nsData = NSData()
119-
let _ = nsData.write(to: remoteUrl, atomically: false) // $ hasPathInjection=110
120-
nsData.write(to: remoteUrl, options: []) // $ hasPathInjection=110
121-
let _ = nsData.write(toFile: remoteString, atomically: false) // $ hasPathInjection=110
122-
nsData.write(toFile: remoteString, options: []) // $ hasPathInjection=110
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
123110

124111
let fm = FileManager()
125-
let _ = fm.contentsOfDirectory(at: remoteUrl, includingPropertiesForKeys: [], options: []) // $ hasPathInjection=110
126-
let _ = fm.contentsOfDirectory(atPath: remoteString) // $ hasPathInjection=110
127-
let _ = fm.enumerator(at: remoteUrl, includingPropertiesForKeys: [], options: [], errorHandler: nil) // $ hasPathInjection=110
128-
let _ = fm.enumerator(atPath: remoteString) // $ hasPathInjection=110
129-
let _ = fm.subpathsOfDirectory(atPath: remoteString) // $ hasPathInjection=110
130-
let _ = fm.subpaths(atPath: remoteString) // $ hasPathInjection=110
131-
fm.createDirectory(at: remoteUrl, withIntermediateDirectories: false, attributes: [:]) // $ hasPathInjection=110
132-
let _ = fm.createDirectory(atPath: remoteString, attributes: [:]) // $ hasPathInjection=110
133-
let _ = fm.createFile(atPath: remoteString, contents: nil, attributes: [:]) // $ hasPathInjection=110
134-
fm.removeItem(at: remoteUrl) // $ hasPathInjection=110
135-
fm.removeItem(atPath: remoteString) // $ hasPathInjection=110
136-
fm.trashItem(at: remoteUrl, resultingItemURL: AutoreleasingUnsafeMutablePointer<NSURL?>()) // $ hasPathInjection=110
137-
let _ = fm.replaceItemAt(remoteUrl, withItemAt: safeUrl, backupItemName: nil, options: []) // $ hasPathInjection=110
138-
let _ = fm.replaceItemAt(safeUrl, withItemAt: remoteUrl, backupItemName: nil, options: []) // $ hasPathInjection=110
139-
fm.replaceItem(at: remoteUrl, withItemAt: safeUrl, backupItemName: nil, options: [], resultingItemURL: AutoreleasingUnsafeMutablePointer<NSURL?>()) // $ hasPathInjection=110
140-
fm.replaceItem(at: safeUrl, withItemAt: remoteUrl, backupItemName: nil, options: [], resultingItemURL: AutoreleasingUnsafeMutablePointer<NSURL?>()) // $ hasPathInjection=110
141-
fm.copyItem(at: remoteUrl, to: safeUrl) // $ hasPathInjection=110
142-
fm.copyItem(at: safeUrl, to: remoteUrl) // $ hasPathInjection=110
143-
fm.copyItem(atPath: remoteString, toPath: "") // $ hasPathInjection=110
144-
fm.copyItem(atPath: "", toPath: remoteString) // $ hasPathInjection=110
145-
fm.moveItem(at: remoteUrl, to: safeUrl) // $ hasPathInjection=110
146-
fm.moveItem(at: safeUrl, to: remoteUrl) // $ hasPathInjection=110
147-
fm.moveItem(atPath: remoteString, toPath: "") // $ hasPathInjection=110
148-
fm.moveItem(atPath: "", toPath: remoteString) // $ hasPathInjection=110
149-
fm.createSymbolicLink(at: remoteUrl, withDestinationURL: safeUrl) // $ hasPathInjection=110
150-
fm.createSymbolicLink(at: safeUrl, withDestinationURL: remoteUrl) // $ hasPathInjection=110
151-
fm.createSymbolicLink(atPath: remoteString, withDestinationPath: "") // $ hasPathInjection=110
152-
fm.createSymbolicLink(atPath: "", withDestinationPath: remoteString) // $ hasPathInjection=110
153-
fm.linkItem(at: remoteUrl, to: safeUrl) // $ hasPathInjection=110
154-
fm.linkItem(at: safeUrl, to: remoteUrl) // $ hasPathInjection=110
155-
fm.linkItem(atPath: remoteString, toPath: "") // $ hasPathInjection=110
156-
fm.linkItem(atPath: "", toPath: remoteString) // $ hasPathInjection=110
157-
let _ = fm.destinationOfSymbolicLink(atPath: remoteString) // $ hasPathInjection=110
158-
let _ = fm.fileExists(atPath: remoteString) // $ hasPathInjection=110
159-
let _ = fm.fileExists(atPath: remoteString, isDirectory: UnsafeMutablePointer<ObjCBool>.init(bitPattern: 0)) // $ hasPathInjection=110
160-
let _ = fm.isReadableFile(atPath: remoteString) // $ hasPathInjection=110
161-
let _ = fm.isWritableFile(atPath: remoteString) // $ hasPathInjection=110
162-
let _ = fm.isDeletableFile(atPath: remoteString) // $ hasPathInjection=110
163-
let _ = fm.componentsToDisplay(forPath: remoteString) // $ hasPathInjection=110
164-
let _ = fm.displayName(atPath: remoteString) // $ hasPathInjection=110
165-
let _ = fm.attributesOfItem(atPath: remoteString) // $ hasPathInjection=110
166-
let _ = fm.attributesOfFileSystem(forPath: remoteString) // $ hasPathInjection=110
167-
fm.setAttributes([:], ofItemAtPath: remoteString) // $ hasPathInjection=110
168-
let _ = fm.contents(atPath: remoteString) // $ hasPathInjection=110
169-
let _ = fm.contentsEqual(atPath: remoteString, andPath: "") // $ hasPathInjection=110
170-
let _ = fm.contentsEqual(atPath: "", andPath: remoteString) // $ hasPathInjection=110
171-
fm.getRelationship(UnsafeMutablePointer<FileManager.URLRelationship>.allocate(capacity: 0), ofDirectoryAt: remoteUrl, toItemAt: safeUrl) // $ hasPathInjection=110
172-
fm.getRelationship(UnsafeMutablePointer<FileManager.URLRelationship>.allocate(capacity: 0), ofDirectoryAt: safeUrl, toItemAt: remoteUrl) // $ hasPathInjection=110
173-
fm.getRelationship(UnsafeMutablePointer<FileManager.URLRelationship>.allocate(capacity: 0), of: FileManager.SearchPathDirectory.none, in: FileManager.SearchPathDomainMask(), toItemAt: remoteUrl) // $ hasPathInjection=110
174-
let _ = fm.changeCurrentDirectoryPath(remoteString) // $ hasPathInjection=110
175-
let _ = fm.unmountVolume(at: remoteUrl, options: [], completionHandler: { _ in }) // $ hasPathInjection=110
176-
let _ = fm.NSHFSTypeOfFile(remoteString) // $ hasPathInjection=110
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
177153
// Deprecated methods
178-
let _ = fm.changeFileAttributes([:], atPath: remoteString) // $ hasPathInjection=110
179-
let _ = fm.fileAttributes(atPath: remoteString, traverseLink: false) // $ hasPathInjection=110
180-
let _ = fm.fileSystemAttributes(atPath: remoteString) // $ hasPathInjection=110
181-
let _ = fm.directoryContents(atPath: remoteString) // $ hasPathInjection=110
182-
let _ = fm.createDirectory(atPath: remoteString, attributes: [:]) // $ hasPathInjection=110
183-
let _ = fm.createSymbolicLink(atPath: remoteString, pathContent: "") // $ hasPathInjection=110
184-
let _ = fm.createSymbolicLink(atPath: "", pathContent: remoteString) // $ hasPathInjection=110
185-
let _ = fm.pathContentOfSymbolicLink(atPath: remoteString) // $ hasPathInjection=110
186-
let _ = fm.replaceItemAtURL(originalItemURL: remoteNsUrl, withItemAtURL: safeNsUrl, backupItemName: nil, options: []) // $ hasPathInjection=110
187-
let _ = fm.replaceItemAtURL(originalItemURL: safeNsUrl, withItemAtURL: remoteNsUrl, backupItemName: nil, options: []) // $ hasPathInjection=110
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
188162
}
189163

190164
func testSanitizers() {
@@ -196,5 +170,5 @@ func testSanitizers() {
196170
if (filePath.lexicallyNormalized().starts(with: FilePath(stringLiteral: "/safe"))) {
197171
fm.contents(atPath: remoteString) // Safe
198172
}
199-
fm.contents(atPath: remoteString) // $ hasPathInjection=191
173+
fm.contents(atPath: remoteString) // $ hasPathInjection=165
200174
}

0 commit comments

Comments
 (0)