nahsra / WebGoat_12_23

Other
0 stars 2 forks source link

Hardened XStream with a converter to prevent exploitation #27

Closed pixeebot[bot] closed 3 months ago

pixeebot[bot] commented 3 months ago

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.

More reading * [https://x-stream.github.io/security.html](https://x-stream.github.io/security.html) * [http://diniscruz.blogspot.com/2013/12/xstream-remote-code-execution-exploit.html](http://diniscruz.blogspot.com/2013/12/xstream-remote-code-execution-exploit.html) * [https://www.contrastsecurity.com/security-influencers/serialization-must-die-act-2-xstream](https://www.contrastsecurity.com/security-influencers/serialization-must-die-act-2-xstream) * [https://x-stream.github.io/CVE-2013-7285.html](https://x-stream.github.io/CVE-2013-7285.html)

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: pixee:java/harden-xstream

sonarcloud[bot] commented 3 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

pixeebot[bot] commented 3 months ago

I'm confident in this change, and the CI checks pass, too!

If you see any reason not to merge this, or you have suggestions for improvements, please let me know!

pixeebot[bot] commented 3 months ago

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!

pixeebot[bot] commented 3 months ago

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.