unified: Add path injection query

unified/ql/lib/ext/legacy-swift.model.yml

@@ -235,6 +235,7 @@ extensions:
       - ["", "Connection", true, "scalar(_:_:)", "", "", "Argument[0]", "database-store", "manual"]
       - ["", "Connection", true, "scalar(_:_:)", "", "", "Argument[1]", "database-store", "manual"]
       - ["", "Connection", true, "init(_:readonly:)", "", "", "Argument[0]", "path-injection", "manual"]
+      - ["", "Connection.Location", true, "uri(_:parameters:)", "", "", "Argument[0]", "path-injection", "manual"]
       - ["", "Database", true, "changePassphrase(_:)", "", "", "Argument[0]", "encryption-key", "manual"]
       - ["", "Database", true, "usePassphrase(_:)", "", "", "Argument[0]", "encryption-key", "manual"]
       - ["", "Database", true, "allStatements(arguments:sql:)", "", "", "Argument[arguments:]", "database-store", "manual"]

unified/ql/src/queries/security/CWE-022/PathInjection.qhelp

@@ -0,0 +1,58 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>Accessing paths controlled by users can expose resources to attackers.</p>
+
+<p>Paths that are naively constructed from data controlled by a user may contain unexpected special characters,
+such as <code>..</code>. Such a path could point to any directory on the file system.</p>
+</overview>
+
+<recommendation>
+
+<p>Validate user input before using it to construct a file path. Ideally, follow these rules:</p>
+
+<ul>
+<li>Do not allow more than a single <code>.</code> character.</li>
+<li>Do not allow directory separators such as <code>/</code> or <code>\</code> (depending on the file system).</li>
+<li>Do not rely on simply replacing problematic sequences such as <code>../</code>. For example, after applying this filter to
+<code>.../...//</code> the resulting string would still be <code>../</code>.</li>
+<li>Use a whitelist of known good patterns.</li>
+</ul>
+
+</recommendation>
+
+<example>
+<p>
+The following code shows two bad examples. 
+</p>
+
+<sample src="PathInjectionBad.swift" />
+
+<p>
+In the first, a file name is read from an HTTP request and then used to access a file. In this case, a malicious response could include a file name that is an absolute path, such as
+<code>"/Applications/(current_application)/Documents/sensitive.data"</code>.
+</p>
+
+<p>
+In the second bad example, it appears that the user is restricted to opening a file within the
+<code>"/Library/Caches"</code> home directory. In this case, a malicious response could contain a file name containing
+special characters. For example, the string <code>"../../Documents/sensitive.data"</code> will result in the code
+reading the file located at <code>"/Applications/(current_application)/Library/Caches/../../Documents/sensitive.data"</code>, 
+which contains users' sensitive data. This file may then be made accessible to an attacker, giving them access to all this data.
+</p>
+
+<p>
+In the following (good) example, the path used to access the file system is normalized <em>before</em> being checked against a
+known prefix. This ensures that regardless of the user input, the resulting path is safe.
+</p>
+
+<sample src="PathInjectionGood.swift" />
+</example>
+
+<references>
+<li>OWASP: <a href="https://owasp.org/www-community/attacks/Path_Traversal">Path Traversal</a>.</li>
+</references>
+</qhelp>

unified/ql/src/queries/security/CWE-022/PathInjection.ql

@@ -0,0 +1,64 @@
+/**
+ * @name Uncontrolled data used in path expression
+ * @description Accessing paths influenced by users can allow an attacker to access unexpected resources.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 7.5
+ * @precision high
+ * @id unified/swift/path-injection
+ * @tags security
+ *       external/cwe/cwe-022
+ *       external/cwe/cwe-023
+ *       external/cwe/cwe-036
+ *       external/cwe/cwe-073
+ *       external/cwe/cwe-099
+ */
+
+import unified
+
+/**
+ * A string that might be a label for a path argument.
+ */
+pragma[inline]
+private predicate pathLikeHeuristic(string label) {
+  label =
+    [
+      "atFile", "atPath", "atDirectory", "toFile", "toPath", "toDirectory", "inFile", "inPath",
+      "inDirectory", "contentsOfFile", "contentsOfPath", "contentsOfDirectory", "filePath",
+      "directory", "directoryPath"
+    ]
+}
+
+predicate heuristicSink(DataFlow::Node node) {
+  node.isIncomingValue(any(Identifier id | id.getValue() = "sqlite3_temp_directory"))
+  or
+  exists(Argument arg |
+    pathLikeHeuristic(arg.getName()) and
+    node.asExpr() = arg.getValue()
+  )
+}
+
+module PathInjectionConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { Models::isSource(node, _) }
+
+  predicate isSink(DataFlow::Node node) {
+    Models::isSink(node, "path-injection") or
+    heuristicSink(node)
+  }
+
+  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { none() }
+
+  predicate isBarrier(DataFlow::Node node) {
+    // TODO: add barriers
+    none()
+  }
+}
+
+module PathInjectionFlow = DataFlow::Global<PathInjectionConfig>;
+
+import PathInjectionFlow::PathGraph
+
+from PathInjectionFlow::PathNode source, PathInjectionFlow::PathNode sink
+where PathInjectionFlow::flowPath(source, sink)
+select sink.getNode(), source, sink, "This path depends on a $@.", source.getNode(),
+  "user-provided value"

unified/ql/src/queries/security/CWE-022/PathInjectionBad.swift

@@ -0,0 +1,10 @@
+let fm = FileManager.default
+let path = try String(contentsOf: URL(string: "http://example.com/")!)
+
+// BAD
+return fm.contents(atPath: path)
+
+// BAD
+if (path.hasPrefix(NSHomeDirectory() + "/Library/Caches")) {
+    return fm.contents(atPath: path)
+}

unified/ql/src/queries/security/CWE-022/PathInjectionGood.swift

@@ -0,0 +1,8 @@
+let fm = FileManager.default
+let path = try String(contentsOf: URL(string: "http://example.com/")!)
+
+// GOOD
+let filePath = FilePath(stringLiteral: path)
+if (filePath.lexicallyNormalized().starts(with: FilePath(stringLiteral: NSHomeDirectory() + "/Library/Caches"))) {
+    return fm.contents(atPath: path)
+}

unified/ql/test/library-tests/mad/test.expected

@@ -15,6 +15,7 @@ isSink
 | test.swift:100:24:100:36 | encryptionKey | encryption-key |
 | test.swift:101:18:101:24 | fileURL | path-injection |
 | test.swift:107:23:107:34 | seedFilePath | path-injection |
+| test.swift:119:29:119:34 | string | path-injection |
 isSource
 | test.swift:4:5:4:27 | String(...) | remote |
 | test.swift:5:5:5:44 | String(...) | remote |

unified/ql/test/library-tests/mad/test.swift

@@ -108,3 +108,13 @@ func testQualifiedConstructors(
         shouldCompactOnLaunch: nil,
         syncConfiguration: nil)
 }
+
+class Connection {
+    enum Location {
+        static func uri(_ path: String, parameters: String) {}
+    }
+}
+
+func testConnectionLocation(string: String) {
+    Connection.Location.uri(string, parameters: "")  // $ isSink=path-injection
+}

unified/ql/test/query-tests/security/CWE-022/PathInjection/CONSISTENCY/CfgConsistency.expected

@@ -0,0 +1,4 @@
+consistencyOverview
+| deadEnd | 1 |
+deadEnd
+| testPathInjection.swift:330:14:330:62 | Entry |