diff options
| author | Jonathan Leitschuh <Jonathan.Leitschuh@gmail.com> | 2022-09-09 03:00:08 +0000 |
|---|---|---|
| committer | Alexander Kriegisch <Alexander@Kriegisch.name> | 2022-09-10 13:44:54 +0200 |
| commit | a792bf94ba3b7c101aeec25fec6f77e328dbc5cd (patch) | |
| tree | a5cbc0db6ae637dee9896c9e50cfdab1a04c40b5 | |
| parent | 208eafba3245da6e3eed1b22269e13c6f9920544 (diff) | |
| download | aspectj-a792bf94ba3b7c101aeec25fec6f77e328dbc5cd.tar.gz aspectj-a792bf94ba3b7c101aeec25fec6f77e328dbc5cd.zip | |
vuln-fix: Partial Path Traversal Vulnerability
This fixes a partial path traversal vulnerability.
Replaces `dir.getCanonicalPath().startsWith(parent.getCanonicalPath())`, which is vulnerable to partial path traversal attacks, with the more secure `dir.getCanonicalFile().toPath().startsWith(parent.getCanonicalFile().toPath())`.
To demonstrate this vulnerability, consider `"/usr/outnot".startsWith("/usr/out")`.
The check is bypassed although `/outnot` is not under the `/out` directory.
It's important to understand that the terminating slash may be removed when using various `String` representations of the `File` object.
For example, on Linux, `println(new File("/var"))` will print `/var`, but `println(new File("/var", "/")` will print `/var/`;
however, `println(new File("/var", "/").getCanonicalPath())` will print `/var`.
Weakness: CWE-22: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')
Severity: Medium
CVSSS: 6.1
Detection: CodeQL & OpenRewrite (https://public.moderne.io/recipes/org.openrewrite.java.security.PartialPathTraversalVulnerability)
Reported-by: Jonathan Leitschuh <Jonathan.Leitschuh@gmail.com>
Signed-off-by: Jonathan Leitschuh <Jonathan.Leitschuh@gmail.com>
Bug-tracker: https://github.com/JLLeitschuh/security-research/issues/13
Co-authored-by: Moderne <team@moderne.io>
2 files changed, 2 insertions, 2 deletions
diff --git a/testing/src/test/java/org/aspectj/testing/util/FileUtil.java b/testing/src/test/java/org/aspectj/testing/util/FileUtil.java index 03b29a85a..103eba59b 100644 --- a/testing/src/test/java/org/aspectj/testing/util/FileUtil.java +++ b/testing/src/test/java/org/aspectj/testing/util/FileUtil.java @@ -477,7 +477,7 @@ public class FileUtil { throws IOException { String path = in.getCanonicalPath(); String parentPath = parent.getCanonicalPath(); - if (!path.startsWith(parentPath)) { + if (!in.getCanonicalFile().toPath().startsWith(parentPath)) { throw new Error("not parent: " + parentPath + " of " + path); } else { path = path.substring(1+parentPath.length()); diff --git a/tests/src/test/java/org/aspectj/systemtest/incremental/tools/AbstractMultiProjectIncrementalAjdeInteractionTestbed.java b/tests/src/test/java/org/aspectj/systemtest/incremental/tools/AbstractMultiProjectIncrementalAjdeInteractionTestbed.java index d9858ce42..827710b26 100644 --- a/tests/src/test/java/org/aspectj/systemtest/incremental/tools/AbstractMultiProjectIncrementalAjdeInteractionTestbed.java +++ b/tests/src/test/java/org/aspectj/systemtest/incremental/tools/AbstractMultiProjectIncrementalAjdeInteractionTestbed.java @@ -159,7 +159,7 @@ public class AbstractMultiProjectIncrementalAjdeInteractionTestbed extends AjdeI try { fileFound = f.getCanonicalPath(); String toRemove = base.getCanonicalPath(); - if (!fileFound.startsWith(toRemove)) { + if (!f.getCanonicalFile().toPath().startsWith(toRemove)) { throw new RuntimeException("eh? " + fileFound + " " + toRemove); } collectionPoint.add(fileFound.substring(toRemove.length() + 1));// +1 captures extra separator |