Skip to content

Conversation

pixeebot[bot]
Copy link

@pixeebot pixeebot bot commented Apr 16, 2024

User description

This change hardens Java deserialization operations against attack. 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;
  ObjectInputStream ois = new ObjectInputStream(is);
+ ObjectInputFilters.enableObjectFilterIfUnprotected(ois);
  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.

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 (codemod ID: pixee:java/harden-java-deserialization)


Description

  • Introduced ObjectInputFilters to enhance security against deserialization attacks in CountrySchemaSql and RainbowFishSerializer.
  • Added java-security-toolkit as a new dependency in the project's main pom.xml and in the specific modules serialized-entity and tolerant-reader.

Changes walkthrough

Relevant files
Enhancement
CountrySchemaSql.java
Harden Java Deserialization in CountrySchemaSql                               

serialized-entity/src/main/java/com/iluwatar/serializedentity/CountrySchemaSql.java

  • Added import for ObjectInputFilters.
  • Inserted a call to ObjectInputFilters.enableObjectFilterIfUnprotected
    to harden deserialization.
  • +2/-0     
    RainbowFishSerializer.java
    Harden Java Deserialization in RainbowFishSerializer                     

    tolerant-reader/src/main/java/com/iluwatar/tolerantreader/RainbowFishSerializer.java

  • Added import for ObjectInputFilters.
  • Inserted a call to ObjectInputFilters.enableObjectFilterIfUnprotected
    to harden deserialization.
  • +2/-0     
    Dependencies
    pom.xml
    Add Java Security Toolkit Dependency in pom.xml                               

    pom.xml

  • Added java-security-toolkit version property.
  • Included java-security-toolkit as a dependency.
  • +7/-0     
    pom.xml
    Include Java Security Toolkit Dependency                                             

    serialized-entity/pom.xml

    • Added java-security-toolkit as a dependency.
    +4/-0     
    pom.xml
    Include Java Security Toolkit Dependency                                             

    tolerant-reader/pom.xml

    • Added java-security-toolkit as a dependency.
    +4/-0     
    💡 Usage Guide

    Checking Your Pull Request

    Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

    Talking to CodeAnt AI

    Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

    @codeant-ai ask: Your question here
    

    This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

    Check Your Repository Health

    To analyze the health of your code repository, visit our dashboard at app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

    <groupId>com.h2database</groupId>
    <artifactId>h2</artifactId>
    </dependency>
    <dependency>
    Copy link
    Author

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    This library holds security tools for protecting Java API calls.

    License: MIT ✅ | Open source ✅ | More facts

    <artifactId>system-lambda</artifactId>
    <version>${system-lambda.version}</version>
    <scope>test</scope>
    </dependency>
    Copy link
    Author

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    This library holds security tools for protecting Java API calls.

    License: MIT ✅ | Open source ✅ | More facts

    <artifactId>junit-jupiter-engine</artifactId>
    <scope>test</scope>
    </dependency>
    <dependency>
    Copy link
    Author

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    This library holds security tools for protecting Java API calls.

    License: MIT ✅ | Open source ✅ | More facts

    <dependency>
    <groupId>io.github.pixee</groupId>
    <artifactId>java-security-toolkit</artifactId>

    Copy link
    Author

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    This library holds security tools for protecting Java API calls.

    License: MIT ✅ | Open source ✅ | More facts

    Copy link

    korbit-ai bot commented Apr 16, 2024

    You’ve installed Korbit to your Github repository but you haven’t created a Korbit account yet! To create your Korbit account and get your PR scans, please visit https://mentor.korbit.ai/dashboard

    Copy link

    The files' contents are under analysis for test generation.

    Copy link

    Hi there! 👋 Thanks for opening a PR. 🎉 To get the most out of Senior Dev, please sign up in our Web App, connect your GitHub account, and add/join your organization Sowhat999. After that, you will receive code reviews beginning on your next opened PR. 🚀

    Copy link

    git-greetings bot commented Apr 16, 2024

    Thanks @pixeebot[bot] for opening this PR!

    For COLLABORATOR only :

    • To add labels, comment on the issue
      /label add label1,label2,label3

    • To remove labels, comment on the issue
      /label remove label1,label2,label3

    Copy link

    @gitginie gitginie bot left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    @pixeebot[bot]
    Thank you for your contribution to this repository! We appreciate your effort in opening pull request.
    Happy coding!

    Copy link

    pr-code-reviewer bot commented Apr 16, 2024

    👋 Hi there!

    Everything looks good!


    Automatically generated with the help of gpt-3.5-turbo.
    Feedback? Please don't hesitate to drop me an email at webber@takken.io.

    Copy link

    Processing PR updates...

    Copy link

    coderabbitai bot commented Apr 16, 2024

    Important

    Auto Review Skipped

    Bot user detected.

    To trigger a single review, invoke the @coderabbitai review command.

    You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


    Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

    Share
    Tips

    Chat

    There are 3 ways to chat with CodeRabbit:

    • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
      • I pushed a fix in commit <commit_id>.
      • Generate unit testing code for this file.
      • Open a follow-up GitHub issue for this discussion.
    • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
      • @coderabbitai generate unit testing code for this file.
      • @coderabbitai modularize this function.
    • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
      • @coderabbitai generate interesting stats about this repository and render them as a table.
      • @coderabbitai show all the console.log statements in this repository.
      • @coderabbitai read src/utils.ts and generate unit testing code.
      • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

    Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

    CodeRabbit Commands (invoked as PR comments)

    • @coderabbitai pause to pause the reviews on a PR.
    • @coderabbitai resume to resume the paused reviews.
    • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
    • @coderabbitai resolve resolve all the CodeRabbit review comments.
    • @coderabbitai help to get help.

    Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

    CodeRabbit Configration File (.coderabbit.yaml)

    • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
    • Please see the configuration documentation for more information.
    • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

    Documentation and Community

    • Visit our Documentation for detailed information on how to use CodeRabbit.
    • Join our Discord Community to get help, request features, and share feedback.
    • Follow us on X/Twitter for updates and announcements.

    Copy link

    instapr bot commented Apr 16, 2024

    Feedback:

    • Implemented protection against deserialization attacks by introducing the java-security-toolkit dependency.
    • Utilized ObjectInputFilters to prevent exploitation through deserialization in CountrySchemaSql.java and RainbowFishSerializer.java.

    Overall, great work on addressing this critical security vulnerability! Well done 👍🏼.

    Copy link

    git-greetings bot commented Apr 16, 2024

    PR Details of @pixeebot[bot] in java-design-patterns :

    OPEN CLOSED TOTAL
    2 1 3

    Copy link

    Potential issues, bugs, and flaws that can introduce unwanted behavior:

    1. /pom.xml:

      • The <versions.java-security-toolkit> property is added but not used anywhere in the file. This can lead to confusion or indicate unused configuration.
    2. /tolerant-reader/pom.xml, /serialized-entity/pom.xml, /serialized-entity/pom.xml:

      • The addition of <groupId>io.github.pixee</groupId> and <artifactId>java-security-toolkit</artifactId> dependencies without specifying a version may cause dependency resolution issues if the version is not locked down appropriately.

    Code suggestions and improvements for better exception handling, logic, standardization, and consistency:

    1. /serialized-entity/src/main/java/com/iluwatar/serializedentity/CountrySchemaSql.java:

      • It is important to handle or log any exceptions that may be thrown by ObjectInputFilters.enableObjectFilterIfUnprotected(ois); to prevent unexpected crashes or silent failures.
    2. /tolerant-reader/src/main/java/com/iluwatar/tolerantreader/RainbowFishSerializer.java:

      • Similar to the previous file, handling or logging exceptions that may be thrown by ObjectInputFilters.enableObjectFilterIfUnprotected(objIn); is crucial for robust error management.

    @gstraccini gstraccini bot requested a review from D0LLi April 16, 2024 06:04
    Comment on lines 111 to 115
    ByteArrayInputStream baos = new ByteArrayInputStream(countryBlob.getBytes(1, (int) countryBlob.length()));
    ObjectInputStream ois = new ObjectInputStream(baos);
    ObjectInputFilters.enableObjectFilterIfUnprotected(ois);
    country = (Country) ois.readObject();
    LOGGER.info("Country: " + country);

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    The code is vulnerable to Java deserialization attacks because it deserializes objects without validating the source or the content of the serialized data. This can lead to various attacks, including arbitrary code execution if the application is processing data from untrusted sources.

    To mitigate this risk, it's recommended to implement a more secure form of serialization or use a library that provides safe deserialization features. Additionally, consider using validation mechanisms such as custom ObjectInputFilters to check the classes being deserialized or the size of the object graph being created.

    Comment on lines 92 to 95
    try (var fileIn = new FileInputStream(filename);
    var objIn = new ObjectInputStream(fileIn)) {
    ObjectInputFilters.enableObjectFilterIfUnprotected(objIn);
    map = (Map<String, String>) objIn.readObject();

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    The use of ObjectInputStream without a proper validation mechanism for deserialized objects can lead to security vulnerabilities, such as arbitrary code execution if the content of the file is malicious. While ObjectInputFilters.enableObjectFilterIfUnprotected(objIn); is an attempt to mitigate this, it's crucial to ensure that the filter is correctly configured to only allow safe classes to be deserialized. Recommended solution is to explicitly check or restrict the classes that can be deserialized by configuring the ObjectInputFilter to only allow known safe classes.

    try (var fileIn = new FileInputStream(filename);
    var objIn = new ObjectInputStream(fileIn)) {
    ObjectInputFilters.enableObjectFilterIfUnprotected(objIn);
    map = (Map<String, String>) objIn.readObject();

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Casting the result of objIn.readObject() directly to (Map<String, String>) without checking can lead to a ClassCastException if the object read is not actually a Map<String, String>. This can be a problem especially when dealing with serialized objects from untrusted sources or when the serialized form might change. It's recommended to perform a type check before casting, or use a safer deserialization method that can handle type mismatches gracefully.

    @codeant-ai codeant-ai bot added @enhancement New feature or request bug_fix labels Apr 16, 2024
    Copy link

    codesyncapp bot commented Apr 16, 2024

    Check out the playback for this Pull Request here.

    Copy link
    Author

    pixeebot bot commented Apr 24, 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!

    Copy link
    Author

    pixeebot bot commented Apr 25, 2024

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

    Copy link
    Author

    pixeebot bot commented May 1, 2024

    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.

    @pixeebot pixeebot bot closed this May 1, 2024
    Copy link

    @gitginie gitginie bot left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    @pixeebot[bot]
    Thank you for your contribution to this repository! We appreciate your effort in closing pull request.
    Happy coding!

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    bug_fix @enhancement New feature or request
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant