mirror of
https://github.com/github/codeql.git
synced 2026-04-29 18:55:14 +02:00
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.
This commit is contained in:
committed by
Antoine Taillefer
parent
bd5619147d
commit
660e6d7085
@@ -1,6 +1,6 @@
|
||||
public class PartialPathTraversalGood {
|
||||
public void example(File dir, File parent) throws IOException {
|
||||
if (!dir.getCanonicalPath().toPath().startsWith(parent.getCanonicalPath().toPath())) {
|
||||
if (!dir.getCanonicalPath().startsWith(parent.getCanonicalPath() + File.separator)) {
|
||||
throw new IOException("Invalid directory: " + dir.getCanonicalPath());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -27,7 +27,7 @@ and not just children of <code>parent</code>, which is a security issue.
|
||||
<p>
|
||||
|
||||
In this example, the <code>if</code> statement checks if <code>parent.getCanonicalPath() + File.separator </code>
|
||||
is a prefix of <code>dir.getCanonicalPath()</code>. Because <code>parent.getCanonicalPath().toPath()</code> is
|
||||
is a prefix of <code>dir.getCanonicalPath()</code>. Because <code>parent.getCanonicalPath() + File.separator</code> is
|
||||
indeed slash-terminated, the user supplying <code>dir</code> can only access children of
|
||||
<code>parent</code>, as desired.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user