Skip to content
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.

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

Hardened XStream with a converter to prevent exploitation #20

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pixeebot[bot]
Copy link

@pixeebot pixeebot bot commented Dec 8, 2024

This change hardens usage of the XStream library to prevent remote code execution attacks.

XStream is a very flexible library, but it has a history of serious vulnerabilities when handling untrusted data because it was never intended for that use case. There are some fundamental issues with the design that make it difficult to make safe when using it by default.

Our change hardens new instances of XStream so that they can't deserialize types that are commonly used in exploits (and never in normal usage) and it looks like this:

+ import io.github.pixee.security.xstream.HardeningConverter;
  XStream xstream = new XStream();
+ xstream.registerConverter(new HardeningConverter());
  return (AcmeObject)xstream.fromXML(xml);

Looking at the history of exploits shows that this change will either stop most exploits or raise the bar of exploitation. If you believe there should be more types added to the denylist, please fill out a ticket with your suggestions.

❌ 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-xstream:1.0.2")
}
Maven
<dependencies>
  <dependency>
    <groupId>io.github.pixee</groupId>
    <artifactId>java-security-toolkit-xstream</artifactId>
    <version>1.0.2</version>
  </dependency>
<dependencies>
More reading

🧚🤖 Powered by Pixeebot

Feedback | Community | Docs | Codemod ID: pixee:java/harden-xstream

@@ -130,6 +130,10 @@
<artifactId>nexus-test-common</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 APIs for hardening XStream operations.

License: MIT ✅ | Open source ✅ | No transitive dependencies ✅ | More facts

@@ -63,7 +63,12 @@
<version>2.6.0-SNAPSHOT</version>
<scope>import</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 APIs for hardening XStream operations.

License: MIT ✅ | Open source ✅ | No transitive dependencies ✅ | More facts

@@ -335,6 +335,10 @@
<artifactId>sisu-pr-testutil</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 APIs for hardening XStream operations.

License: MIT ✅ | Open source ✅ | No transitive dependencies ✅ | More facts

<dependency>
<groupId>io.github.pixee</groupId>
<artifactId>java-security-toolkit-xstream</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 APIs for hardening XStream operations.

License: MIT ✅ | Open source ✅ | No transitive dependencies ✅ | More facts

@@ -88,6 +88,10 @@
<artifactId>com.noelios.restlet.ext.jetty</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 APIs for hardening XStream operations.

License: MIT ✅ | Open source ✅ | No transitive dependencies ✅ | More facts

@@ -164,7 +165,11 @@
</exclusion>
</exclusions>
</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 APIs for hardening XStream operations.

License: MIT ✅ | Open source ✅ | No transitive dependencies ✅ | More facts

@@ -170,6 +170,10 @@
<version>${cargo.version}</version>
<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 APIs for hardening XStream operations.

License: MIT ✅ | Open source ✅ | No transitive dependencies ✅ | More facts

@@ -64,7 +65,11 @@
<version>2.6.0-SNAPSHOT</version>
<scope>import</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 APIs for hardening XStream operations.

License: MIT ✅ | Open source ✅ | No transitive dependencies ✅ | More facts

@@ -176,6 +176,10 @@
<artifactId>nexus-indexer-lucene-plugin</artifactId>
<version>${nexus.version}</version>
</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 APIs for hardening XStream operations.

License: MIT ✅ | Open source ✅ | No transitive dependencies ✅ | More facts

@@ -49,7 +49,11 @@
<version>2.6.0-SNAPSHOT</version>
<scope>import</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 APIs for hardening XStream operations.

License: MIT ✅ | Open source ✅ | No transitive dependencies ✅ | More facts

Copy link
Author

pixeebot bot commented Dec 16, 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 Dec 17, 2024

Merging this change just means the code won't be able to deserialize types like java.lang.Runtime or java.lang.ProcessBuilder -- which is always a good thing. Not having a control like this is how this exploit against Jenkins happen.

If there are other concerns about this change, I'd love to hear about them!

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.

0 participants