This is an automated email from the ASF dual-hosted git repository.
jamesbognar pushed a commit to branch juneau-9.2.1-branch
in repository https://gitbox.apache.org/repos/asf/juneau.git
The following commit(s) were added to refs/heads/juneau-9.2.1-branch by this
push:
new b91bce1617 Harden DirectoryResource file path resolution
b91bce1617 is described below
commit b91bce161713998fc9953b713263cd894af2931d
Author: James Bognar <[email protected]>
AuthorDate: Mon Aug 10 10:50:08 2026 -0400
Harden DirectoryResource file path resolution
Resolve requested paths against a canonicalized root and reject paths that
resolve outside it. Adds a regression test covering the rejected cases.
---
.../org/apache/juneau/commons/utils/FileUtils.java | 70 +++++
.../microservice/resources/DirectoryResource.java | 27 +-
.../DirectoryResource_PathTraversal_Test.java | 306 +++++++++++++++++++++
3 files changed, 397 insertions(+), 6 deletions(-)
diff --git
a/juneau-core/juneau-commons/src/main/java/org/apache/juneau/commons/utils/FileUtils.java
b/juneau-core/juneau-commons/src/main/java/org/apache/juneau/commons/utils/FileUtils.java
index 5887c5eab9..76be28de8c 100644
---
a/juneau-core/juneau-commons/src/main/java/org/apache/juneau/commons/utils/FileUtils.java
+++
b/juneau-core/juneau-commons/src/main/java/org/apache/juneau/commons/utils/FileUtils.java
@@ -24,6 +24,7 @@ import static org.apache.juneau.commons.utils.Utils.*;
import java.io.*;
import java.nio.file.*;
+import java.util.*;
/**
* File utilities.
@@ -31,6 +32,75 @@ import java.nio.file.*;
*/
public class FileUtils {
+ // Shared message text — kept identical across boundary-check sites so
attackers cannot
+ // distinguish "rejected by pre-existence check" from "rejected by
symlink check".
+ private static final String MSG_pathEscape = "Path escapes configured
root directory.";
+
+ /**
+ * Resolves a user-supplied path against a configured root directory,
rejecting any path that
+ * escapes the root via {@code ..} segments, absolute paths, or
out-of-root symlinks.
+ *
+ * <p>
+ * Both {@code rootDir} and the resolved target are canonicalized via
{@link Path#toRealPath()}
+ * (with {@link Path#normalize()}) so symlinks are handled
deterministically, then
+ * {@code target.startsWith(root)} is asserted before returning.
+ *
+ * <p>
+ * Boundary violation always wins over pre-existence: a path that
escapes the root is rejected
+ * with {@link IllegalArgumentException} (callers should map to {@code
403 Forbidden}), while a
+ * non-existent in-root target returns {@link Optional#empty()}
(callers should map to
+ * {@code 404 Not Found}).
+ *
+ * @param rootDir The configured root directory. Must not be
<jk>null</jk>.
+ * @param userPath The user-supplied path relative to {@code rootDir}.
May be <jk>null</jk> or
+ * empty, in which case the resolved root itself is returned.
+ * @return The resolved {@link File} if it exists inside {@code
rootDir}, else empty.
+ * @throws IllegalArgumentException If the resolved target escapes
{@code rootDir}, or the path
+ * string is invalid. Callers should map to {@code 403 Forbidden}.
+ */
+ public static Optional<File> resolveSafely(File rootDir, String
userPath) {
+ assertArgNotNull("rootDir", rootDir);
+ var root = canonicalizeRoot(rootDir.toPath());
+ if (userPath == null || userPath.isEmpty())
+ return Optional.of(root.toFile());
+ Path target;
+ try {
+ target = root.resolve(userPath).normalize();
+ } catch (@SuppressWarnings("unused") InvalidPathException e) {
+ throw new IllegalArgumentException(MSG_pathEscape);
+ }
+ if (! target.startsWith(root))
+ throw new IllegalArgumentException(MSG_pathEscape);
+ var f = target.toFile();
+ if (! f.exists())
+ return Optional.empty();
+ try {
+ if (! target.toRealPath().startsWith(root))
+ throw new
IllegalArgumentException(MSG_pathEscape);
+ } catch (@SuppressWarnings("unused") NoSuchFileException e) {
+ return Optional.empty();
+ } catch (IOException e) {
+ throw new RuntimeException("Could not canonicalize '" +
target + "'", e);
+ }
+ return Optional.of(f);
+ }
+
+ /**
+ * Resolves {@code rootDir} via {@link Path#toRealPath()} so a
symlinked root is normalized once,
+ * falling back to {@link Path#toAbsolutePath()} + {@link
Path#normalize()} if the directory does
+ * not exist at call time.
+ *
+ * @param rootDir The root directory path.
+ * @return The canonicalized root path.
+ */
+ private static Path canonicalizeRoot(Path rootDir) {
+ try {
+ return rootDir.toRealPath();
+ } catch (@SuppressWarnings("unused") IOException e) {
+ return rootDir.toAbsolutePath().normalize();
+ }
+ }
+
/**
* Creates a file if it doesn't already exist using {@link
File#createNewFile()}.
*
diff --git
a/juneau-microservice/juneau-microservice-core/src/main/java/org/apache/juneau/microservice/resources/DirectoryResource.java
b/juneau-microservice/juneau-microservice-core/src/main/java/org/apache/juneau/microservice/resources/DirectoryResource.java
index 7a3b98a42b..66e1da257d 100755
---
a/juneau-microservice/juneau-microservice-core/src/main/java/org/apache/juneau/microservice/resources/DirectoryResource.java
+++
b/juneau-microservice/juneau-microservice-core/src/main/java/org/apache/juneau/microservice/resources/DirectoryResource.java
@@ -26,6 +26,7 @@ import java.util.*;
import org.apache.juneau.annotation.*;
import org.apache.juneau.bean.*;
+import org.apache.juneau.commons.utils.FileUtils;
import org.apache.juneau.config.*;
import org.apache.juneau.html.annotation.*;
import org.apache.juneau.http.annotation.*;
@@ -312,13 +313,27 @@ public class DirectoryResource extends BasicRestServlet {
throw new Forbidden("Could not delete file {0}",
f.getAbsolutePath());
}
+ /**
+ * Resolves a request path against {@link #rootDir} with a
canonical-path boundary check so
+ * {@code ..} segments, absolute paths, and symlink-out cannot escape
the configured root.
+ *
+ * <p>
+ * Applied at this single funnel because every public operation on this
resource (view,
+ * download, delete, upload, info) resolves the user-supplied path
through this method.
+ * Delegates to {@link FileUtils#resolveSafely(File, String)}: boundary
violation → 403
+ * (Forbidden), non-existent target → 404 (NotFound).
+ *
+ * @param path The user-supplied path relative to the root directory.
+ * @return The resolved file.
+ * @throws NotFound If the resolved file does not exist inside the root
directory.
+ */
private File getFile(String path) throws NotFound {
- if (path == null)
- return rootDir;
- var f = new File(rootDir.getAbsolutePath() + '/' + path);
- if (f.exists())
- return f;
- throw new NotFound("File not found.");
+ try {
+ return FileUtils.resolveSafely(rootDir, path)
+ .orElseThrow(() -> new NotFound("File not
found."));
+ } catch (IllegalArgumentException e) {
+ throw new Forbidden(e.getMessage());
+ }
}
/**
diff --git
a/juneau-utest/src/test/java/org/apache/juneau/microservice/resources/DirectoryResource_PathTraversal_Test.java
b/juneau-utest/src/test/java/org/apache/juneau/microservice/resources/DirectoryResource_PathTraversal_Test.java
new file mode 100644
index 0000000000..e89fc771ac
--- /dev/null
+++
b/juneau-utest/src/test/java/org/apache/juneau/microservice/resources/DirectoryResource_PathTraversal_Test.java
@@ -0,0 +1,306 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.juneau.microservice.resources;
+
+import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assumptions.*;
+
+import java.io.*;
+import java.nio.file.*;
+
+import org.apache.juneau.*;
+import org.apache.juneau.config.*;
+import org.apache.juneau.rest.annotation.*;
+import org.apache.juneau.rest.mock.*;
+import org.junit.jupiter.api.*;
+import org.junit.jupiter.api.io.*;
+
+/**
+ * Regression tests for path-traversal hardening in {@link
DirectoryResource#getFile(String)}.
+ *
+ * <p>
+ * The pre-fix implementation did {@code new File(rootDir.getAbsolutePath() +
'/' + path)} and only
+ * verified {@link File#exists()} — so {@code ..} segments, absolute paths,
and symlinks pointing
+ * outside the configured root could all read files the JVM had access to.
Each test below asserts
+ * the post-fix behavior: paths that escape the configured root are rejected
with 403 (Forbidden);
+ * paths that stay inside the root continue to work.
+ *
+ * <p>
+ * The funnel for every public operation on this resource (view, download,
delete, upload, info) is
+ * the private {@code getFile(String)} method, so the fix is applied once and
verified across each
+ * operation surface here.
+ *
+ * @since 9.2.1
+ */
+class DirectoryResource_PathTraversal_Test extends TestBase {
+
+ @TempDir
+ static Path tempDir;
+
+ static Path rootDir;
+ static Path outsideSecret;
+ static Path symlinkInside;
+ static Path symlinkEscape;
+
+ @BeforeAll
+ static void setup() throws Exception {
+ rootDir = tempDir.resolve("dir-root");
+ Files.createDirectories(rootDir);
+ Files.writeString(rootDir.resolve("inside.txt"), "INSIDE_ROOT");
+
+ var nestedDir = rootDir.resolve("a/b");
+ Files.createDirectories(nestedDir);
+ Files.writeString(nestedDir.resolve("nested.txt"),
"NESTED_INSIDE");
+
+ // File OUTSIDE the configured root — what the path-traversal
attack tries to read.
+ outsideSecret = tempDir.resolve("outside-secret.txt");
+ Files.writeString(outsideSecret, "AUDIT_OUTSIDE_SECRET");
+
+ // File OUTSIDE the configured root that the upload-traversal
attack tries to overwrite/create.
+ Files.writeString(tempDir.resolve("outside-upload-target.txt"),
"ORIGINAL_OUTSIDE");
+
+ // Best-effort symlinks. Skip via assumption on filesystems
that don't support them.
+ try {
+ symlinkInside = rootDir.resolve("link-to-inside.txt");
+ Files.createSymbolicLink(symlinkInside,
rootDir.resolve("inside.txt"));
+ } catch (UnsupportedOperationException | IOException e) {
+ symlinkInside = null;
+ }
+ try {
+ symlinkEscape = rootDir.resolve("link-to-outside");
+ Files.createSymbolicLink(symlinkEscape, outsideSecret);
+ } catch (UnsupportedOperationException | IOException e) {
+ symlinkEscape = null;
+ }
+ }
+
+ /**
+ * Test resource with a no-arg constructor so {@link MockRestClient}
can instantiate it via
+ * reflection. Points at the test's {@link #rootDir} and enables every
operation so the boundary
+ * check is exercised on each surface (view, download, delete, upload,
info).
+ */
+ @Rest(
+ allowedMethodParams="*"
+ )
+ public static class TestDirResource extends DirectoryResource {
+ private static final long serialVersionUID = 1L;
+
+ public TestDirResource() throws Exception {
+ super(buildConfig());
+ }
+
+ private static Config buildConfig() {
+ var cfg = Config.create().memStore().build();
+ cfg.set(DIRECTORY_RESOURCE_rootDir, rootDir.toString());
+ cfg.set(DIRECTORY_RESOURCE_allowViews, "true");
+ cfg.set(DIRECTORY_RESOURCE_allowUploads, "true");
+ cfg.set(DIRECTORY_RESOURCE_allowDeletes, "true");
+ return cfg;
+ }
+ }
+
+ @SuppressWarnings({
+ "resource" // Closeable resources in tests are intentionally
unassigned; closing is handled by test infrastructure.
+ })
+ private static MockRestClient buildClient() {
+ return MockRestClient.buildLax(TestDirResource.class);
+ }
+
+
//-----------------------------------------------------------------------------------------------------------------
+ // Baseline — non-traversing requests still work
+
//-----------------------------------------------------------------------------------------------------------------
+
+ @SuppressWarnings({
+ "resource" // Closeable resources in tests are intentionally
unassigned; closing is handled by test infrastructure.
+ })
+ @Test void t01_normalAccess_view() throws Exception {
+ try (var c = buildClient()) {
+ c.request("VIEW", "/inside.txt").run()
+ .assertStatus(200)
+ .assertContent().is("INSIDE_ROOT");
+ }
+ }
+
+
//-----------------------------------------------------------------------------------------------------------------
+ // direct ../ traversal across each operation surface (view, download,
delete)
+
//-----------------------------------------------------------------------------------------------------------------
+
+ @Test void t02_directTraversal_GET_returns403() throws Exception {
+ try (var c = buildClient()) {
+ @SuppressWarnings({
+ "resource" // Closeable resources in tests are
intentionally unassigned; closing is handled by test infrastructure.
+ })
+ var status =
c.get("/../outside-secret.txt").run().getStatusCode();
+ assertEquals(403, status, "GET /../outside-secret.txt
must be rejected (path escapes root)");
+ }
+ }
+
+ @Test void t03_methodVIEW_traversal_returns403() throws Exception {
+ try (var c = buildClient()) {
+ @SuppressWarnings({
+ "resource" // Closeable resources in tests are
intentionally unassigned; closing is handled by test infrastructure.
+ })
+ var resp =
c.get("/../outside-secret.txt?method=VIEW").run();
+ assertEquals(403, resp.getStatusCode(), "GET
/../outside-secret.txt?method=VIEW must be rejected");
+
assertFalse(resp.getContent().asString().contains("AUDIT_OUTSIDE_SECRET"),
+ "Response body must not leak the outside-root
secret");
+ }
+ }
+
+ @Test void t04_methodDOWNLOAD_traversal_returns403() throws Exception {
+ try (var c = buildClient()) {
+ @SuppressWarnings({
+ "resource" // Closeable resources in tests are
intentionally unassigned; closing is handled by test infrastructure.
+ })
+ var resp =
c.get("/../outside-secret.txt?method=DOWNLOAD").run();
+ assertEquals(403, resp.getStatusCode(), "GET
/../outside-secret.txt?method=DOWNLOAD must be rejected");
+
assertFalse(resp.getContent().asString().contains("AUDIT_OUTSIDE_SECRET"),
+ "Response body must not leak the outside-root
secret");
+ }
+ }
+
+ @Test void t05_verbVIEW_traversal_returns403() throws Exception {
+ try (var c = buildClient()) {
+ @SuppressWarnings({
+ "resource" // Closeable resources in tests are
intentionally unassigned; closing is handled by test infrastructure.
+ })
+ var status = c.request("VIEW",
"/../outside-secret.txt").run().getStatusCode();
+ assertEquals(403, status, "VIEW /../outside-secret.txt
must be rejected");
+ }
+ }
+
+ @Test void t06_verbDOWNLOAD_traversal_returns403() throws Exception {
+ try (var c = buildClient()) {
+ @SuppressWarnings({
+ "resource" // Closeable resources in tests are
intentionally unassigned; closing is handled by test infrastructure.
+ })
+ var status = c.request("DOWNLOAD",
"/../outside-secret.txt").run().getStatusCode();
+ assertEquals(403, status, "DOWNLOAD
/../outside-secret.txt must be rejected");
+ }
+ }
+
+ @Test void t07_nestedTraversal_returns403() throws Exception {
+ try (var c = buildClient()) {
+ @SuppressWarnings({
+ "resource" // Closeable resources in tests are
intentionally unassigned; closing is handled by test infrastructure.
+ })
+ var status =
c.get("/a/b/../../../outside-secret.txt").run().getStatusCode();
+ assertEquals(403, status, "GET
/a/b/../../../outside-secret.txt must be rejected");
+ }
+ }
+
+
//-----------------------------------------------------------------------------------------------------------------
+ // Absolute path / URL-encoded variants
+
//-----------------------------------------------------------------------------------------------------------------
+
+ @Test void t08_absolutePathInRequest_isNotTreatedAsFilesystemAbsolute()
throws Exception {
+ // "/etc/passwd" arrives as a relative segment under root → 404
(no such file), or 403 on
+ // platforms where it resolves absolute. Either is a non-leak
outcome.
+ try (var c = buildClient()) {
+ @SuppressWarnings({
+ "resource" // Closeable resources in tests are
intentionally unassigned; closing is handled by test infrastructure.
+ })
+ var status = c.get("/etc/passwd").run().getStatusCode();
+ assertTrue(status == 403 || status == 404, "Status must
be 403 or 404, was: " + status);
+ }
+ }
+
+ @Test void t09_urlEncodedTraversal_doesNotLeak() throws Exception {
+ // Container/HttpClient may URL-decode before our handler, or
reject "%2e%2e" outright. Either
+ // is acceptable as long as the outside-root secret is NOT
returned.
+ try (var c = buildClient()) {
+ @SuppressWarnings({
+ "resource" // Closeable resources in tests are
intentionally unassigned; closing is handled by test infrastructure.
+ })
+ var resp = c.get("/%2e%2e/outside-secret.txt").run();
+
assertFalse(resp.getContent().asString().contains("AUDIT_OUTSIDE_SECRET"),
+ "URL-encoded traversal must not leak the
outside-root secret. Status was: " + resp.getStatusCode());
+ assertNotEquals(200, resp.getStatusCode(),
+ "URL-encoded traversal must not return 200.
Status was: " + resp.getStatusCode());
+ }
+ }
+
+
//-----------------------------------------------------------------------------------------------------------------
+ // Upload / delete traversal (uploads + deletes enabled in
TestDirResource)
+
//-----------------------------------------------------------------------------------------------------------------
+
+ @Test void t10_uploadTraversal_does_not_create_outside_root() throws
Exception {
+ var outsideUpload =
tempDir.resolve("outside-uploaded-by-test.txt");
+ assertFalse(Files.exists(outsideUpload), "Pre-condition: upload
target must not exist");
+
+ try (var c = buildClient()) {
+ @SuppressWarnings({
+ "resource" // Closeable resources in tests are
intentionally unassigned; closing is handled by test infrastructure.
+ })
+ var status = c.put("/../outside-uploaded-by-test.txt",
"ATTACK_PAYLOAD").run().getStatusCode();
+ // Post-fix: boundary check fires first → 403 (or 404
legacy "must exist before PUT").
+ // Either way, the outside file must NOT be created.
+ assertTrue(status == 403 || status == 404,
+ "PUT to outside-root path must be rejected with
403/404, was: " + status);
+ assertFalse(Files.exists(outsideUpload),
+ "Outside-root file must NOT be created by PUT
traversal");
+ }
+ }
+
+ @Test void t11_deleteTraversal_does_not_delete_outside_root() throws
Exception {
+ assertTrue(Files.exists(outsideSecret), "Pre-condition: outside
secret must exist");
+
+ try (var c = buildClient()) {
+ @SuppressWarnings({
+ "resource" // Closeable resources in tests are
intentionally unassigned; closing is handled by test infrastructure.
+ })
+ var status =
c.delete("/../outside-secret.txt").run().getStatusCode();
+ assertEquals(403, status, "DELETE
/../outside-secret.txt must be rejected with 403");
+ assertTrue(Files.exists(outsideSecret),
+ "Outside-root file must NOT be deleted by
DELETE traversal");
+ assertEquals("AUDIT_OUTSIDE_SECRET",
Files.readString(outsideSecret),
+ "Outside-root file content must be unchanged
after DELETE traversal attempt");
+ }
+ }
+
+
//-----------------------------------------------------------------------------------------------------------------
+ // Symlink handling
+
//-----------------------------------------------------------------------------------------------------------------
+
+ @SuppressWarnings({
+ "resource" // Closeable resources in tests are intentionally
unassigned; closing is handled by test infrastructure.
+ })
+ @Test void t12_symlinkInsideRoot_is_followed() throws Exception {
+ assumeTrue(symlinkInside != null, "Filesystem does not support
symbolic links");
+
+ try (var c = buildClient()) {
+ c.request("VIEW", "/link-to-inside.txt").run()
+ .assertStatus(200)
+ .assertContent().is("INSIDE_ROOT");
+ }
+ }
+
+ @Test void t13_symlinkEscapesRoot_is_rejected() throws Exception {
+ assumeTrue(symlinkEscape != null, "Filesystem does not support
symbolic links");
+
+ try (var c = buildClient()) {
+ @SuppressWarnings({
+ "resource" // Closeable resources in tests are
intentionally unassigned; closing is handled by test infrastructure.
+ })
+ var resp = c.request("VIEW", "/link-to-outside").run();
+ assertEquals(403, resp.getStatusCode(),
+ "Symlink to outside-root must be rejected with
403 (post-existence boundary check)");
+
assertFalse(resp.getContent().asString().contains("AUDIT_OUTSIDE_SECRET"),
+ "Symlink-escape response must not leak the
outside-root secret");
+ }
+ }
+}
