Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,8 @@
**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.

## 2026-10-25 - Replace Wide Wildcards in ValidatingObjectInputStream with Explicit Class Allow-List
**Vulnerability:** In `DiskUtils.readSerializable`, `ValidatingObjectInputStream.accept()` used wide wildcards (`java.lang.*`, `java.util.*`, `me.desair.tus.server.*`) which permitted deserialization of any class within those packages, exposing potential gadget chains for insecure deserialization attacks.
**Learning:** Broad package wildcards in `ValidatingObjectInputStream` reduce the protection against insecure deserialization, allowing arbitrary classes within allowed packages to be instantiated during deserialization.
**Prevention:** Always construct explicit, concrete class allow-lists when using `ValidatingObjectInputStream.accept(...)` by supplying expected class literal references rather than wide package-level wildcards.
52 changes: 49 additions & 3 deletions src/main/java/me/desair/tus/server/upload/disk/DiskUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@
import static java.nio.file.StandardOpenOption.WRITE;

import java.io.BufferedOutputStream;
import java.io.EOFException;
import java.io.IOException;
import java.io.InputStream;
import java.io.InvalidClassException;
import java.io.ObjectOutput;
import java.io.ObjectOutputStream;
import java.io.OutputStream;
import java.io.Serializable;
import java.io.StreamCorruptedException;
import java.nio.channels.FileChannel;
import java.nio.channels.FileLock;
import java.nio.file.AtomicMoveNotSupportedException;
Expand All @@ -22,8 +25,21 @@
import java.nio.file.StandardCopyOption;
import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.FileTime;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.Objects;
import java.util.TreeMap;
import java.util.TreeSet;
import java.util.UUID;
import me.desair.tus.server.checksum.ChecksumAlgorithm;
import me.desair.tus.server.upload.UploadId;
import me.desair.tus.server.upload.UploadInfo;
import me.desair.tus.server.upload.UploadType;
import me.desair.tus.server.util.Utils;
import org.apache.commons.codec.binary.Base64;
import org.apache.commons.io.serialization.ValidatingObjectInputStream;
Expand Down Expand Up @@ -290,10 +306,40 @@ public static <T> T readSerializable(Path path, Class<T> clazz) throws IOExcepti
if (path != null && Files.exists(path)) {
try (InputStream is = Files.newInputStream(path);
ValidatingObjectInputStream ois = new ValidatingObjectInputStream(is)) {
ois.accept("java.lang.*", "java.util.*", "me.desair.tus.server.*");
if (clazz != null) {
ois.accept(clazz);
}
ois.accept(
UploadInfo.class,
UploadId.class,
UploadType.class,
ChecksumAlgorithm.class,
String.class,
Long.class,
Integer.class,
Boolean.class,
Number.class,
Object.class,
Enum.class,
Date.class,
UUID.class,
ArrayList.class,
LinkedList.class,
TreeMap.class,
HashMap.class,
HashSet.class,
TreeSet.class,
String[].class,
Object[].class,
Arrays.asList().getClass(),
Collections.unmodifiableList(new ArrayList<>()).getClass(),
Collections.emptyList().getClass());
info = clazz.cast(ois.readObject());
} catch (ClassNotFoundException | java.io.EOFException | java.io.StreamCorruptedException e) {
// File may be corrupted due to unexpected server shutdown
} catch (ClassNotFoundException
| InvalidClassException
| EOFException
| StreamCorruptedException e) {
// File may be corrupted due to unexpected server shutdown or contain an unallowed class
log.warn("Unable to read serializable file {}: {}", path, e.getMessage());
info = null;
}
Expand Down
20 changes: 20 additions & 0 deletions src/test/java/me/desair/tus/server/upload/disk/DiskUtilsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,13 @@ public void testReadAndWriteSerializable() throws Exception {
assertThat(result, is(notNullValue()));
assertThat(result.getValue(), is("test-value-123"));

// Unauthorized class object serialization attempt when expecting UploadInfo.class
Path unallowedFile = storagePath.resolve("unallowed.bin");
DiskUtils.writeSerializable(new UnallowedSerializable("untrusted"), unallowedFile);
assertThat(
DiskUtils.readSerializable(unallowedFile, me.desair.tus.server.upload.UploadInfo.class),
is(nullValue()));

// Corrupted file
Path corrupted = storagePath.resolve("corrupted.bin");
Files.write(corrupted, "not a valid serialized object".getBytes());
Expand Down Expand Up @@ -344,4 +351,17 @@ public String getValue() {
return value;
}
}

public static class UnallowedSerializable implements Serializable {
private static final long serialVersionUID = 1L;
private final String payload;

public UnallowedSerializable(String payload) {
this.payload = payload;
}

public String getPayload() {
return payload;
}
}
}
31 changes: 31 additions & 0 deletions src/test/java/me/desair/tus/server/util/UtilsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,23 @@ public void readSerializableWithValidFile() throws Exception {
Files.deleteIfExists(testFile);
}

@Test
public void readSerializableWithUnallowedClass() throws Exception {
Path unallowedFile = storagePath.resolve("unallowed-" + UUID.randomUUID());
UnallowedTestSerializable original = new UnallowedTestSerializable("malicious-payload");

Utils.writeSerializable(original, unallowedFile);

// Should return null due to ValidatingObjectInputStream class validation rejection when reading
// as UploadInfo
me.desair.tus.server.upload.UploadInfo result =
Utils.readSerializable(unallowedFile, me.desair.tus.server.upload.UploadInfo.class);

assertThat(result, is(nullValue()));

Files.deleteIfExists(unallowedFile);
}

@Test
public void readSerializableWithCorruptedFile() throws Exception {
Path corruptedFile = storagePath.resolve("corrupted-" + UUID.randomUUID());
Expand Down Expand Up @@ -1109,4 +1126,18 @@ public String getValue() {
return value;
}
}

/** Unallowed serializable class for testing rejection. */
public static class UnallowedTestSerializable implements Serializable {
private static final long serialVersionUID = 1L;
private final String payload;

public UnallowedTestSerializable(String payload) {
this.payload = payload;
}

public String getPayload() {
return payload;
}
}
}
Loading