diff --git a/.jules/sentinel.md b/.jules/sentinel.md index c2c07f0f..49397a8c 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -23,3 +23,7 @@ **Vulnerability:** The `CorsRequestHandler.java` implementation blindly read the `Origin` header from incoming HTTP requests and reflected its exact value back in the `Access-Control-Allow-Origin` response header. **Learning:** Blindly mirroring the request's origin bypasses the intended security of CORS restrictions and can lead to arbitrary cross-origin data exposure, effectively negating the same-origin policy enforcement by browsers. Moreover, doing so without `Vary: Origin` could lead to cache poisoning in intermediate CDNs. **Prevention:** Hardcode public API endpoint CORS rules to return the explicitly permissive `*` (wildcard) instead of reflecting the user's string input. +## 2026-09-29 - Prevent Path Traversal in LeaseFileLockingService and LeaseFileMutex +**Vulnerability:** A Path Traversal vulnerability existed in `LeaseFileLockingService` and `LeaseFileMutex` where the `UploadId` was blindly resolved against the base storage directory to construct `.lock`, `.stop`, and `.mutex` paths. +**Learning:** Similar to the previous issue in `AbstractDiskBasedService`, any paths constructed from `UploadId` input must be properly normalized and constrained to the base storage directory, as unvalidated `UploadId`s could lead to out-of-bounds file access or creation. +**Prevention:** Always validate that constructed file paths derived from user input or external identifiers remain within their intended boundary directory by using `path.normalize().toAbsolutePath().startsWith(storagePath.normalize().toAbsolutePath())`. Ensure this check is consistently applied across all file-based services. diff --git a/src/main/java/me/desair/tus/server/upload/disk/LeaseFileLockingService.java b/src/main/java/me/desair/tus/server/upload/disk/LeaseFileLockingService.java index 90b17823..f144f25d 100644 --- a/src/main/java/me/desair/tus/server/upload/disk/LeaseFileLockingService.java +++ b/src/main/java/me/desair/tus/server/upload/disk/LeaseFileLockingService.java @@ -374,18 +374,28 @@ boolean atomicEvictExpiredLock(Path lockDirPath) { } } - Path getLockDirPath(UploadId id) { + Path getLockDirPath(final UploadId id) { if (id == null) { return null; } - return storagePath.resolve(id.toString() + ".lock"); + Path path = storagePath.resolve(id.toString() + ".lock"); + if (!path.normalize().toAbsolutePath().startsWith(storagePath.normalize().toAbsolutePath())) { + throw new IllegalArgumentException( + "Upload ID is not valid and would result in a path traversal"); + } + return path; } - Path getStopFilePath(UploadId id) { + Path getStopFilePath(final UploadId id) { if (id == null) { return null; } - return storagePath.resolve(id.toString() + ".stop"); + Path path = storagePath.resolve(id.toString() + ".stop"); + if (!path.normalize().toAbsolutePath().startsWith(storagePath.normalize().toAbsolutePath())) { + throw new IllegalArgumentException( + "Upload ID is not valid and would result in a path traversal"); + } + return path; } private synchronized void initStoragePath() { diff --git a/src/main/java/me/desair/tus/server/upload/disk/LeaseFileMutex.java b/src/main/java/me/desair/tus/server/upload/disk/LeaseFileMutex.java index 1d7909e4..e6618fbd 100644 --- a/src/main/java/me/desair/tus/server/upload/disk/LeaseFileMutex.java +++ b/src/main/java/me/desair/tus/server/upload/disk/LeaseFileMutex.java @@ -45,11 +45,17 @@ public LeaseFileMutex(Path lockDirPath) { * @param storagePath Path to the base locks storage directory * @param uploadId The upload identifier */ - public LeaseFileMutex(Path storagePath, UploadId uploadId) { - this.mutexDir = - (storagePath != null && uploadId != null) - ? storagePath.resolve(uploadId.toString() + ".mutex") - : null; + public LeaseFileMutex(final Path storagePath, final UploadId uploadId) { + if (storagePath != null && uploadId != null) { + Path path = storagePath.resolve(uploadId.toString() + ".mutex"); + if (!path.normalize().toAbsolutePath().startsWith(storagePath.normalize().toAbsolutePath())) { + throw new IllegalArgumentException( + "Upload ID is not valid and would result in a path traversal"); + } + this.mutexDir = path; + } else { + this.mutexDir = null; + } this.acquired = tryAcquire(); }