diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 0000000..e80d8de --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,7 @@ +## 2026-06-23 - Path Traversal Vulnerability in Disk Storage + +**Vulnerability:** The application allowed path traversal in the disk storage component when resolving upload IDs. Specifically, `storagePath.resolve(id.toString())` was used without checking if the resulting path was still within the intended storage directory. + +**Learning:** When resolving paths using user-controlled input (like an upload ID), it is insufficient to simply rely on `Path.resolve`. If the input contains `../` or absolute paths, it could escape the intended boundaries. + +**Prevention:** Always normalize the resolved path and explicitly check that it starts with the normalized base directory using `uploadPath.normalize().startsWith(storagePath.normalize())`. Throw an exception if the check fails. diff --git a/src/main/java/me/desair/tus/server/upload/disk/AbstractDiskBasedService.java b/src/main/java/me/desair/tus/server/upload/disk/AbstractDiskBasedService.java index 0c12472..5b0f1fd 100644 --- a/src/main/java/me/desair/tus/server/upload/disk/AbstractDiskBasedService.java +++ b/src/main/java/me/desair/tus/server/upload/disk/AbstractDiskBasedService.java @@ -37,7 +37,18 @@ protected Path getPathInStorageDirectory(UploadId id) { if (id == null) { return null; } else { - return storagePath.resolve(id.toString()); + Path uploadPath = + storagePath + .resolve( + id.getOriginalObject() != null + ? id.getOriginalObject().toString() + : id.toString()) + .normalize(); + if (!uploadPath.startsWith(storagePath.normalize())) { + throw new IllegalArgumentException( + "The upload ID cannot point to a path outside the storage directory"); + } + return uploadPath; } } diff --git a/src/test/java/me/desair/tus/server/upload/disk/AbstractDiskBasedServiceTest.java b/src/test/java/me/desair/tus/server/upload/disk/AbstractDiskBasedServiceTest.java new file mode 100644 index 0000000..49a4667 --- /dev/null +++ b/src/test/java/me/desair/tus/server/upload/disk/AbstractDiskBasedServiceTest.java @@ -0,0 +1,49 @@ +package me.desair.tus.server.upload.disk; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.fail; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import me.desair.tus.server.upload.UploadId; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +public class AbstractDiskBasedServiceTest { + + private Path storagePath; + private AbstractDiskBasedService service; + + @Before + public void setUp() throws IOException { + storagePath = Files.createTempDirectory("tus-test-storage"); + service = new AbstractDiskBasedService(storagePath.toString()) {}; + } + + @After + public void tearDown() throws IOException { + org.apache.commons.io.FileUtils.deleteDirectory(storagePath.toFile()); + } + + @Test + public void getPathInStorageDirectoryValidId() { + UploadId id = new UploadId("valid-id-123"); + Path resolved = service.getPathInStorageDirectory(id); + assertThat(resolved, is(storagePath.resolve("valid-id-123").normalize())); + } + + @Test + public void getPathInStorageDirectoryPathTraversal() { + try { + UploadId id = new UploadId("../../../../../etc/passwd"); + service.getPathInStorageDirectory(id); + fail("Expected IllegalArgumentException to be thrown for path traversal attempt"); + } catch (IllegalArgumentException e) { + assertThat( + e.getMessage(), is("The upload ID cannot point to a path outside the storage directory")); + } + } +}