Swift: Remove the sources instead (more general solution).

Author: geoffw0

swift/ql/lib/codeql/swift/frameworks/StandardLibrary/FileManager.qll

@@ -7,18 +7,23 @@ private import codeql.swift.dataflow.ExternalFlow
 
 /**
  * A model for `FileManager` members that are flow sources.
+ *
+ * Note that functions returning paths on the file system have been removed
+ * from this model. Though they are in principle tainted by the data on the
+ * local filesystem, in practice we've found results from them almost always
+ * have little value.
  */
 private class FileManagerSource extends SourceModelCsv {
   override predicate row(string row) {
     row =
       [
-        ";FileManager;true;contentsOfDirectory(at:includingPropertiesForKeys:options:);;;ReturnValue;local",
-        ";FileManager;true;contentsOfDirectory(atPath:);;;ReturnValue;local",
-        ";FileManager;true;directoryContents(atPath:);;;ReturnValue;local",
-        ";FileManager;true;subpathsOfDirectory(atPath:);;;ReturnValue;local",
-        ";FileManager;true;subpaths(atPath:);;;ReturnValue;local",
-        ";FileManager;true;destinationOfSymbolicLink(atPath:);;;ReturnValue;local",
-        ";FileManager;true;pathContentOfSymbolicLink(atPath:);;;ReturnValue;local",
+        //";FileManager;true;contentsOfDirectory(at:includingPropertiesForKeys:options:);;;ReturnValue;local",
+        //";FileManager;true;contentsOfDirectory(atPath:);;;ReturnValue;local",
+        //";FileManager;true;directoryContents(atPath:);;;ReturnValue;local",
+        //";FileManager;true;subpathsOfDirectory(atPath:);;;ReturnValue;local",
+        //";FileManager;true;subpaths(atPath:);;;ReturnValue;local",
+        //";FileManager;true;destinationOfSymbolicLink(atPath:);;;ReturnValue;local",
+        //";FileManager;true;pathContentOfSymbolicLink(atPath:);;;ReturnValue;local",
         ";FileManager;true;contents(atPath:);;;ReturnValue;local"
       ]
   }

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

@@ -64,12 +64,5 @@ private class CommandInjectionDefaultBarrier extends CommandInjectionBarrier {
   CommandInjectionDefaultBarrier() {
     // any numeric type
     this.asExpr().getType().getUnderlyingType().getABaseType*().getName() = "Numeric"
-    or
-    // we get poor results when the tainted data is a directory list, such as
-    // the result of `FileMananger.contentsOfDirectory` and similar functions.
-    exists(CallExpr ce |
-      ce.getStaticTarget().getName().matches(["%directory%", "%Directory%"]) and
-      this.asExpr() = ce
-    )
   }
 }

swift/ql/test/library-tests/dataflow/flowsources/FlowSourcesInline.expected

@@ -1,2 +1,2 @@
-failures
 testFailures
+failures

swift/ql/test/library-tests/dataflow/flowsources/filemanager.swift

@@ -34,15 +34,15 @@ class FileManager : NSObject {
 func testFileHandle(fm: FileManager, url: URL, path: String) {
 	do
 	{
-		let contents1 = try fm.contentsOfDirectory(at: url, includingPropertiesForKeys: nil) // $ source=local
-		let contents2 = try fm.contentsOfDirectory(atPath: path) // $ source=local
-		let contents3 = fm.directoryContents(atPath: path)! // $ source=local
+		let contents1 = try fm.contentsOfDirectory(at: url, includingPropertiesForKeys: nil)
+		let contents2 = try fm.contentsOfDirectory(atPath: path)
+		let contents3 = fm.directoryContents(atPath: path)!
 
-		let subpaths1 = try fm.subpathsOfDirectory(atPath: path) // $ source=local
-		let subpaths2 = fm.subpaths(atPath: path)! // $ source=local
+		let subpaths1 = try fm.subpathsOfDirectory(atPath: path)
+		let subpaths2 = fm.subpaths(atPath: path)!
 
-		let link1 = try fm.destinationOfSymbolicLink(atPath: path) // $ source=local
-		let link2 = fm.pathContentOfSymbolicLink(atPath: path)! // $ source=local
+		let link1 = try fm.destinationOfSymbolicLink(atPath: path)
+		let link2 = fm.pathContentOfSymbolicLink(atPath: path)!
 
 		let data = fm.contents(atPath: path)! // $ source=local
 	} catch {