Skip to content

Commit 629db4f

Browse files
committed
unified: Add no-op path injection and port test suite
1 parent ccaf910 commit 629db4f

7 files changed

Lines changed: 713 additions & 0 deletions

File tree

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>Accessing paths controlled by users can expose resources to attackers.</p>
8+
9+
<p>Paths that are naively constructed from data controlled by a user may contain unexpected special characters,
10+
such as <code>..</code>. Such a path could point to any directory on the file system.</p>
11+
</overview>
12+
13+
<recommendation>
14+
15+
<p>Validate user input before using it to construct a file path. Ideally, follow these rules:</p>
16+
17+
<ul>
18+
<li>Do not allow more than a single <code>.</code> character.</li>
19+
<li>Do not allow directory separators such as <code>/</code> or <code>\</code> (depending on the file system).</li>
20+
<li>Do not rely on simply replacing problematic sequences such as <code>../</code>. For example, after applying this filter to
21+
<code>.../...//</code> the resulting string would still be <code>../</code>.</li>
22+
<li>Use a whitelist of known good patterns.</li>
23+
</ul>
24+
25+
</recommendation>
26+
27+
<example>
28+
<p>
29+
The following code shows two bad examples.
30+
</p>
31+
32+
<sample src="PathInjectionBad.swift" />
33+
34+
<p>
35+
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
36+
<code>"/Applications/(current_application)/Documents/sensitive.data"</code>.
37+
</p>
38+
39+
<p>
40+
In the second bad example, it appears that the user is restricted to opening a file within the
41+
<code>"/Library/Caches"</code> home directory. In this case, a malicious response could contain a file name containing
42+
special characters. For example, the string <code>"../../Documents/sensitive.data"</code> will result in the code
43+
reading the file located at <code>"/Applications/(current_application)/Library/Caches/../../Documents/sensitive.data"</code>,
44+
which contains users' sensitive data. This file may then be made accessible to an attacker, giving them access to all this data.
45+
</p>
46+
47+
<p>
48+
In the following (good) example, the path used to access the file system is normalized <em>before</em> being checked against a
49+
known prefix. This ensures that regardless of the user input, the resulting path is safe.
50+
</p>
51+
52+
<sample src="PathInjectionGood.swift" />
53+
</example>
54+
55+
<references>
56+
<li>OWASP: <a href="https://owasp.org/www-community/attacks/Path_Traversal">Path Traversal</a>.</li>
57+
</references>
58+
</qhelp>
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/**
2+
* @name Uncontrolled data used in path expression
3+
* @description Accessing paths influenced by users can allow an attacker to access unexpected resources.
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @security-severity 7.5
7+
* @precision high
8+
* @id unified/swift/path-injection
9+
* @tags security
10+
* external/cwe/cwe-022
11+
* external/cwe/cwe-023
12+
* external/cwe/cwe-036
13+
* external/cwe/cwe-073
14+
* external/cwe/cwe-099
15+
*/
16+
17+
import unified
18+
19+
module PathInjectionConfig implements DataFlow::ConfigSig {
20+
predicate isSource(DataFlow::Node node) { none() }
21+
22+
predicate isSink(DataFlow::Node node) { none() }
23+
24+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { none() }
25+
26+
predicate isBarrier(DataFlow::Node node) { none() }
27+
}
28+
29+
module PathInjectionFlow = DataFlow::Global<PathInjectionConfig>;
30+
31+
import PathInjectionFlow::PathGraph
32+
33+
from PathInjectionFlow::PathNode source, PathInjectionFlow::PathNode sink
34+
where PathInjectionFlow::flowPath(source, sink)
35+
select sink.getNode(), source, sink, "This path depends on a $@.", source.getNode(),
36+
"user-provided value"
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
let fm = FileManager.default
2+
let path = try String(contentsOf: URL(string: "http://example.com/")!)
3+
4+
// BAD
5+
return fm.contents(atPath: path)
6+
7+
// BAD
8+
if (path.hasPrefix(NSHomeDirectory() + "/Library/Caches")) {
9+
return fm.contents(atPath: path)
10+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
let fm = FileManager.default
2+
let path = try String(contentsOf: URL(string: "http://example.com/")!)
3+
4+
// GOOD
5+
let filePath = FilePath(stringLiteral: path)
6+
if (filePath.lexicallyNormalized().starts(with: FilePath(stringLiteral: NSHomeDirectory() + "/Library/Caches"))) {
7+
return fm.contents(atPath: path)
8+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
#select
2+
edges
3+
nodes
4+
subpaths
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
query: queries/Security/CWE-022/PathInjection.ql
2+
postprocess:
3+
- utils/test/InlineExpectationsTestQuery.ql

0 commit comments

Comments
 (0)