Skip to content

Conversation

@pixee-latio
Copy link

@pixee-latio pixee-latio bot commented Dec 23, 2024

Remediation

This change fixes "Deserialization" (id = java/Deserialization) identified by Snyk.

Details

This change fixes Java deserialization vulnerabilities. Even a simple operation like an object deserialization is an opportunity to yield control of your system to an attacker. In fact, without specific, non-default protections, any object deserialization call can lead to arbitrary code execution. The JavaDoc now even says:

Deserialization of untrusted data is inherently dangerous and should be avoided.

Let's discuss the attack. In Java, types can customize how they should be deserialized by specifying a readObject() method like this real example from an old version of Spring:

static class MethodInvokeTypeProvider implements TypeProvider {
    private final TypeProvider provider;
    private final String methodName;

    private void readObject(ObjectInputStream inputStream) {
        inputStream.defaultReadObject();
        Method method = ReflectionUtils.findMethod(
                this.provider.getType().getClass(),
                this.methodName
        );
        this.result = ReflectionUtils.invokeMethod(method,this.provider.getType());
    }
}

Reflecting on this code reveals a terrifying conclusion. If an attacker presents this object to be deserialized by your app, the runtime will take a class and a method name from the attacker and then call them. Note that an attacker can provide any serliazed type -- it doesn't have to be the one you're expecting, and it will still deserialize.

Attackers can repurpose the logic of selected types within the Java classpath (called "gadgets") and chain them together to achieve arbitrary remote code execution. There are a limited number of publicly known gadgets that can be used for attack, and our change simply inserts an ObjectInputFilter into the ObjectInputStream to prevent them from being used.

+ import io.github.pixee.security.ObjectInputFilters.createSafeObjectInputStream;
- ObjectInputStream ois = new ObjectInputStream(is);
+ ObjectInputStream ois = createSafeObjectInputStream(is);
  AcmeObject acme = (AcmeObject)ois.readObject();

This is a tough vulnerability class to understand, but it is deadly serious. It offers the highest impact possible (remote code execution), it's a common vulnerability (it's in the OWASP Top 10), and exploitation is easy enough that automated exploitation is possible. It's best to remove deserialization entirely, but our protections is effective against all known exploitation strategies.

❌ The following packages couldn't be installed automatically, probably because the dependency manager is unsupported. Please install them manually:

Gradle
dependencies {
  implementation("io.github.pixee:java-security-toolkit:1.2.1")
}
Maven
<dependencies>
  <dependency>
    <groupId>io.github.pixee</groupId>
    <artifactId>java-security-toolkit</artifactId>
    <version>1.2.1</version>
  </dependency>
<dependencies>
More reading

I have additional improvements ready for this repo! If you want to see them, leave the comment:

@pixeebot next

... and I will open a new PR right away!

🧚🤖 Powered by Pixeebot

Feedback | Community | Docs | Codemod ID: snyk:java/deserialization

@dryrunsecurity
Copy link

DryRun Security Summary

The code change addresses a critical security vulnerability in a Spring Boot application by replacing direct ObjectInputStream usage with a safer method that uses a custom ObjectInputFilter to prevent remote code execution through unsafe deserialization.

Expand for full summary

Summary:

This code change addresses a critical security vulnerability related to unsafe deserialization in a Spring Boot application. The original code used a direct ObjectInputStream to deserialize incoming byte[] data, which can lead to remote code execution (RCE) vulnerabilities if the input is not properly sanitized. The code change introduces the use of createSafeObjectInputStream from the io.github.pixee.security.ObjectInputFilters class, which creates an ObjectInputStream that uses a custom ObjectInputFilter to restrict the types of objects that can be deserialized. This effectively mitigates the unsafe deserialization vulnerability and prevents the application from being vulnerable to RCE attacks through unsafe deserialization. This change is a significant improvement from a security perspective and aligns with recommended security practices when dealing with deserialization of untrusted data.

Files Changed:

  • insecure-java/src/main/java/com/example/insecurejava/UnsafeDeserializationController.java: The code change in this file addresses the unsafe deserialization vulnerability by replacing the direct use of ObjectInputStream with the createSafeObjectInputStream method, which creates an ObjectInputStream with a custom ObjectInputFilter to restrict the types of objects that can be deserialized. This change effectively mitigates the risk of remote code execution through unsafe deserialization.

Code Analysis

We ran 9 analyzers against 1 file and 0 analyzers had findings. 9 analyzers had no findings.

View PR in the DryRun Dashboard.

@pixee-latio
Copy link
Author

pixee-latio bot commented Dec 27, 2024

I'm confident in this change, but I'm not a maintainer of this project. Do you see any reason not to merge it?

If this change was not helpful, or you have suggestions for improvements, please let me know!

@pixee-latio
Copy link
Author

pixee-latio bot commented Jan 1, 2025

Just a friendly ping to remind you about this change. If there are concerns about it, we'd love to hear about them!

@pixee-latio
Copy link
Author

pixee-latio bot commented Jan 7, 2025

This change may not be a priority right now, so I'll close it. If there was something I could have done better, please let me know!

You can also customize me to make sure I'm working with you in the way you want.

@pixee-latio pixee-latio bot closed this Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant