Skip to content

Java: fix bug in partial path traversal #21734

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
owen-mc wants to merge 5 commits into
base: main
Choose a base branch
from
+37 −19
Open

Java: fix bug in partial path traversal #21734

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dca7046
Make inline expectation comments specify query
owen-mc Apr 18, 2026
54f3b27
Use inline expectations with second test
owen-mc Apr 18, 2026
55401ef
Add SPURIOUS test for `+= File.separator`
owen-mc Apr 18, 2026
6a4fa3a
Fix bug so `+= File.separator` is recognized
owen-mc Apr 18, 2026
3bf9483
Add change note
owen-mc Apr 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions java/ql/lib/change-notes/2026-04-18-partial-path-traversal-fix.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The `java/partial-path-traversal` and `java/partial-path-traversal-from-remote` queries now correctly recognize file separator appends using `+=`.
7 changes: 5 additions & 2 deletions java/ql/lib/semmle/code/java/security/PartialPathTraversal.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,11 @@ private class CharacterLiteralFileSeparatorExpr extends FileSeparatorExpr, Chara
CharacterLiteralFileSeparatorExpr() { this.getValue() = "/" or this.getValue() = "\\" }
}

private class FileSeparatorAppend extends AddExpr {
FileSeparatorAppend() { this.getRightOperand() instanceof FileSeparatorExpr }
private class FileSeparatorAppend extends BinaryExpr {
FileSeparatorAppend() {
this.(AddExpr).getRightOperand() instanceof FileSeparatorExpr or
this.(AssignAddExpr).getRightOperand() instanceof FileSeparatorExpr
}
}

private predicate isSafe(Expr expr) {
Expand Down
5 changes: 4 additions & 1 deletion java/ql/test/query-tests/security/CWE-023/semmle/tests/PartialPathTraversal.qlref
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
Security/CWE/CWE-023/PartialPathTraversal.ql
query: Security/CWE/CWE-023/PartialPathTraversal.ql
postprocess:
- utils/test/PrettyPrintModels.ql
- utils/test/InlineExpectationsTestQuery.ql
40 changes: 24 additions & 16 deletions java/ql/test/query-tests/security/CWE-023/semmle/tests/PartialPathTraversalTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@

public class PartialPathTraversalTest {
public void esapiExample(File parent) throws IOException {
if (!dir().getCanonicalPath().startsWith(parent.getCanonicalPath())) { // $ Alert
if (!dir().getCanonicalPath().startsWith(parent.getCanonicalPath())) { // $ Alert[java/partial-path-traversal-from-remote] Alert[java/partial-path-traversal]
throw new IOException("Invalid directory: " + dir().getCanonicalPath());
}
}

@SuppressWarnings("ResultOfMethodCallIgnored")
void foo1(File parent) throws IOException {
(dir().getCanonicalPath()).startsWith((parent.getCanonicalPath())); // $ Alert
(dir().getCanonicalPath()).startsWith((parent.getCanonicalPath())); // $ Alert[java/partial-path-traversal-from-remote] Alert[java/partial-path-traversal]
}

void foo2(File parent) throws IOException {
Expand All @@ -29,42 +29,42 @@ void foo2(File parent) throws IOException {

void foo3(File parent) throws IOException {
String parentPath = parent.getCanonicalPath();
if (!dir().getCanonicalPath().startsWith(parentPath)) { // $ Alert
if (!dir().getCanonicalPath().startsWith(parentPath)) { // $ Alert[java/partial-path-traversal-from-remote] Alert[java/partial-path-traversal]
throw new IOException("Invalid directory: " + dir().getCanonicalPath());
}
}

void foo4() throws IOException {
if (!dir().getCanonicalPath().startsWith("/usr" + "/dir")) { // $ Alert
if (!dir().getCanonicalPath().startsWith("/usr" + "/dir")) { // $ Alert[java/partial-path-traversal-from-remote] Alert[java/partial-path-traversal]
throw new IOException("Invalid directory: " + dir().getCanonicalPath());
}
}

void foo5(File parent) throws IOException {
String canonicalPath = dir().getCanonicalPath();
if (!canonicalPath.startsWith(parent.getCanonicalPath())) { // $ Alert
if (!canonicalPath.startsWith(parent.getCanonicalPath())) { // $ Alert[java/partial-path-traversal-from-remote] Alert[java/partial-path-traversal]
throw new IOException("Invalid directory: " + dir().getCanonicalPath());
}
}

void foo6(File parent) throws IOException {
String canonicalPath = dir().getCanonicalPath();
if (!canonicalPath.startsWith(parent.getCanonicalPath())) { // $ Alert
if (!canonicalPath.startsWith(parent.getCanonicalPath())) { // $ Alert[java/partial-path-traversal-from-remote] Alert[java/partial-path-traversal]
throw new IOException("Invalid directory: " + dir().getCanonicalPath());
}
String canonicalPath2 = dir().getCanonicalPath();
if (!canonicalPath2.startsWith(parent.getCanonicalPath())) { // $ Alert
if (!canonicalPath2.startsWith(parent.getCanonicalPath())) { // $ Alert[java/partial-path-traversal-from-remote] Alert[java/partial-path-traversal]
throw new IOException("Invalid directory: " + dir().getCanonicalPath());
}
}

void foo7(File dir, File parent) throws IOException {
String canonicalPath = dir().getCanonicalPath();
String canonicalPath2 = dir().getCanonicalPath();
if (!canonicalPath.startsWith(parent.getCanonicalPath())) { // $ Alert
if (!canonicalPath.startsWith(parent.getCanonicalPath())) { // $ Alert[java/partial-path-traversal-from-remote] Alert[java/partial-path-traversal]
throw new IOException("Invalid directory: " + dir().getCanonicalPath());
}
if (!canonicalPath2.startsWith(parent.getCanonicalPath())) { // $ Alert
if (!canonicalPath2.startsWith(parent.getCanonicalPath())) { // $ Alert[java/partial-path-traversal-from-remote] Alert[java/partial-path-traversal]
throw new IOException("Invalid directory: " + dir().getCanonicalPath());
}
}
Expand All @@ -75,7 +75,7 @@ File getChild() {

void foo8(File parent) throws IOException {
String canonicalPath = getChild().getCanonicalPath();
if (!canonicalPath.startsWith(parent.getCanonicalPath())) {
if (!canonicalPath.startsWith(parent.getCanonicalPath())) { // $ Alert[java/partial-path-traversal-from-remote] Alert[java/partial-path-traversal]
Copy link

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inline expectation on this line includes Alert[java/partial-path-traversal-from-remote], but the PartialPathTraversalFromRemoteTest.expected file does not contain a result at this location (it only expects java/partial-path-traversal here). This will make the from-remote inline expectations fail unless the query is also expected to alert here. Consider restricting this marker to only Alert[java/partial-path-traversal] (or updating the from-remote expected output if the query behavior is intended to change).

This issue also appears on line 231 of the same file.

Suggested change
if (!canonicalPath.startsWith(parent.getCanonicalPath())) { // $ Alert[java/partial-path-traversal-from-remote] Alert[java/partial-path-traversal]
if (!canonicalPath.startsWith(parent.getCanonicalPath())) { // $ Alert[java/partial-path-traversal]

Copilot uses AI. Check for mistakes.
throw new IOException("Invalid directory: " + getChild().getCanonicalPath());
}
}
Expand All @@ -94,18 +94,18 @@ void foo10(File parent) throws IOException {

void foo11(File parent) throws IOException {
String parentCanonical = parent.getCanonicalPath();
if (!dir().getCanonicalPath().startsWith(parentCanonical)) { // $ Alert
if (!dir().getCanonicalPath().startsWith(parentCanonical)) { // $ Alert[java/partial-path-traversal-from-remote] Alert[java/partial-path-traversal]
throw new IOException("Invalid directory: " + dir().getCanonicalPath());
}
}

void foo12(File parent) throws IOException {
String parentCanonical = parent.getCanonicalPath();
String parentCanonical2 = parent.getCanonicalPath();
if (!dir().getCanonicalPath().startsWith(parentCanonical)) { // $ Alert
if (!dir().getCanonicalPath().startsWith(parentCanonical)) { // $ Alert[java/partial-path-traversal-from-remote] Alert[java/partial-path-traversal]
throw new IOException("Invalid directory: " + dir().getCanonicalPath());
}
if (!dir().getCanonicalPath().startsWith(parentCanonical2)) { // $ Alert
if (!dir().getCanonicalPath().startsWith(parentCanonical2)) { // $ Alert[java/partial-path-traversal-from-remote] Alert[java/partial-path-traversal]
throw new IOException("Invalid directory: " + dir().getCanonicalPath());
}
}
Expand Down Expand Up @@ -173,7 +173,7 @@ void foo18(File dir, File parent, boolean branch) throws IOException {

void foo19(File parent) throws IOException {
String parentCanonical = parent.getCanonicalPath() + "/potato";
if (!dir().getCanonicalPath().startsWith(parentCanonical)) { // $ Alert
if (!dir().getCanonicalPath().startsWith(parentCanonical)) { // $ Alert[java/partial-path-traversal-from-remote] Alert[java/partial-path-traversal]
throw new IOException("Invalid directory: " + dir().getCanonicalPath());
}
}
Expand All @@ -191,7 +191,7 @@ InputStream foo20() {
String filePath = sb.toString();
File encodedFile = new File(filePath);
try {
if (!encodedFile.getCanonicalPath().startsWith(cacheDir.getCanonicalPath())) { // $ Alert
if (!encodedFile.getCanonicalPath().startsWith(cacheDir.getCanonicalPath())) { // $ Alert[java/partial-path-traversal-from-remote] Alert[java/partial-path-traversal]
return null;
}
return Files.newInputStream(encodedFile.toPath());
Expand All @@ -209,7 +209,7 @@ void foo21(File parent) throws IOException {

void foo22(File dir2, File parent, boolean conditional) throws IOException {
String canonicalPath = conditional ? dir().getCanonicalPath() : dir2.getCanonicalPath();
if (!canonicalPath.startsWith(parent.getCanonicalPath())) { // $ Alert
if (!canonicalPath.startsWith(parent.getCanonicalPath())) { // $ Alert[java/partial-path-traversal-from-remote] Alert[java/partial-path-traversal]
throw new IOException("Invalid directory: " + dir().getCanonicalPath());
}
}
Expand All @@ -228,6 +228,14 @@ void foo24(File parent) throws IOException {
}
}

void foo25(File parent) throws IOException {
String path = parent.getCanonicalPath();
path += File.separator;
if (!dir().getCanonicalPath().startsWith(path)) {
throw new IOException("Invalid directory: " + dir().getCanonicalPath());
}
}

public void doesNotFlagOptimalSafeVersion(File parent) throws IOException {
if (!dir().toPath().normalize().startsWith(parent.toPath())) { // Safe
throw new IOException("Path traversal attempt: " + dir().getCanonicalPath());
Expand Down
Loading