Skip to content

Commit 660e6d7

Browse files
committed
Fix partial path traversal Java example
The Java recommendation example for the "Partial path traversal vulnerability from remote" query doesn't seem right to me. Indeed, the following statement doesn't compile, since `dir.getCanonicalPath()` returns a String: ``` dir.getCanonicalPath().toPath() ``` Maybe the author wanted to state `dir.getCanonicalFile().toPath()`, which would compile, but is useless compared to `dir.getCanonicalPath()`. Moreover, `parent.getCanonicalFile().toPath()` or `parent.getCanonicalPath()` will **not** be slash-terminated, contrary to what the description says. From what I can see (and test), the correct fix is to concatenate `File.separator` to the parent canonical path.
1 parent bd56191 commit 660e6d7

File tree

2 files changed

+2
-2
lines changed

2 files changed

+2
-2
lines changed

java/ql/src/Security/CWE/CWE-023/PartialPathTraversalGood.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
public class PartialPathTraversalGood {
22
public void example(File dir, File parent) throws IOException {
3-
if (!dir.getCanonicalPath().toPath().startsWith(parent.getCanonicalPath().toPath())) {
3+
if (!dir.getCanonicalPath().startsWith(parent.getCanonicalPath() + File.separator)) {
44
throw new IOException("Invalid directory: " + dir.getCanonicalPath());
55
}
66
}

java/ql/src/Security/CWE/CWE-023/PartialPathTraversalRemainder.inc.qhelp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ and not just children of <code>parent</code>, which is a security issue.
2727
<p>
2828

2929
In this example, the <code>if</code> statement checks if <code>parent.getCanonicalPath() + File.separator </code>
30-
is a prefix of <code>dir.getCanonicalPath()</code>. Because <code>parent.getCanonicalPath().toPath()</code> is
30+
is a prefix of <code>dir.getCanonicalPath()</code>. Because <code>parent.getCanonicalPath() + File.separator</code> is
3131
indeed slash-terminated, the user supplying <code>dir</code> can only access children of
3232
<code>parent</code>, as desired.
3333

0 commit comments

Comments
 (0)