Skip to content

🛡️ Sentinel: [CRITICAL] Fix insecure deserialization in UploadInfo parsing #98

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tomdesair wants to merge 1 commit into from
+8 −2
Closed

🛡️ Sentinel: [CRITICAL] Fix insecure deserialization in UploadInfo parsing #98

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions .jules/sentinel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
## 2024-06-28 - Insecure Deserialization in UploadInfo parsing
**Vulnerability:** The application reads serialized UploadInfo files from disk using a standard `ObjectInputStream`. This allows an attacker to inject arbitrary serialized objects if they have the ability to write to the storage directory, potentially leading to Remote Code Execution (RCE) via insecure deserialization.
**Learning:** `ObjectInputStream` inherently deserializes any class that implements `Serializable` and is available on the classpath. Even when expecting a specific class like `UploadInfo`, the object stream will instantiate the malicious class before the cast occurs.
**Prevention:** Use `ValidatingObjectInputStream` from `commons-io` to enforce a strict allowlist of classes that can be deserialized. This ensures only expected classes (e.g., `UploadInfo` and its valid fields like `UploadType`, `UploadId`) are processed, rejecting unexpected classes before instantiation.
6 changes: 4 additions & 2 deletions src/main/java/me/desair/tus/server/util/Utils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import jakarta.servlet.http.HttpServletRequest;
import java.io.BufferedOutputStream;
import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutput;
import java.io.ObjectOutputStream;
import java.io.OutputStream;
Expand All @@ -22,6 +21,7 @@
import java.util.regex.Pattern;
import me.desair.tus.server.HttpHeader;
import me.desair.tus.server.checksum.ChecksumAlgorithm;
import org.apache.commons.io.serialization.ValidatingObjectInputStream;
import org.apache.commons.lang3.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -83,7 +83,9 @@ public static <T> T readSerializable(Path path, Class<T> clazz) throws IOExcepti
// Lock will be released when the channel is closed
if (lockFileShared(channel) != null) {

try (ObjectInputStream ois = new ObjectInputStream(Channels.newInputStream(channel))) {
try (ValidatingObjectInputStream ois =
new ValidatingObjectInputStream(Channels.newInputStream(channel))) {
ois.accept("java.lang.*", "java.util.*", "me.desair.tus.**", "[*");
info = clazz.cast(ois.readObject());
} catch (ClassNotFoundException
| java.io.EOFException
Expand Down
Loading