From ac2be35390c528d07e7896d1191864d244b92859 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sun, 25 Oct 2026 10:18:38 +0000 Subject: [PATCH] Fix insecure deserialization via ValidatingObjectInputStream strict class filtering Replace package-level wildcards in ValidatingObjectInputStream with an explicit allow-list of allowed classes to eliminate insecure object deserialization vectors. Co-authored-by: tomdesair <14034630+tomdesair@users.noreply.github.com> --- .jules/sentinel.md | 5 ++ .../tus/server/upload/disk/DiskUtils.java | 52 +++++++++++++++++-- .../tus/server/upload/disk/DiskUtilsTest.java | 20 +++++++ .../me/desair/tus/server/util/UtilsTest.java | 31 +++++++++++ 4 files changed, 105 insertions(+), 3 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 49397a8c..e161a663 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -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. diff --git a/src/main/java/me/desair/tus/server/upload/disk/DiskUtils.java b/src/main/java/me/desair/tus/server/upload/disk/DiskUtils.java index ebf46fd5..35cda4f0 100644 --- a/src/main/java/me/desair/tus/server/upload/disk/DiskUtils.java +++ b/src/main/java/me/desair/tus/server/upload/disk/DiskUtils.java @@ -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; @@ -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; @@ -290,10 +306,40 @@ public static T readSerializable(Path path, Class 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; } diff --git a/src/test/java/me/desair/tus/server/upload/disk/DiskUtilsTest.java b/src/test/java/me/desair/tus/server/upload/disk/DiskUtilsTest.java index 729c5c2a..c8c214c7 100644 --- a/src/test/java/me/desair/tus/server/upload/disk/DiskUtilsTest.java +++ b/src/test/java/me/desair/tus/server/upload/disk/DiskUtilsTest.java @@ -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()); @@ -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; + } + } } diff --git a/src/test/java/me/desair/tus/server/util/UtilsTest.java b/src/test/java/me/desair/tus/server/util/UtilsTest.java index 757261d4..471f651a 100644 --- a/src/test/java/me/desair/tus/server/util/UtilsTest.java +++ b/src/test/java/me/desair/tus/server/util/UtilsTest.java @@ -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()); @@ -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; + } + } }