Hi Dinis

I just read the article. That's bad. Very bad. I have always advocated
using XStream as a good fit to perform remote communication in the way you
describe, which I now see is a very dangerous thing.

I'm not sure what we can do to resolve this, but I hope we can brainstorm
some ideas to do our part to ensure systems using XStream directly or
indirectly are safe.

In an ideal world, the process of serialization and deserialization should
purely be a case of transferring data and should have no side effects. That
is, serializing and deserializing should only ever invoke trusted code in
the XStream library and never any other code outside of XStream (methods,
constructors, static blocks).

If XStream were to be used with just the ReflectionConverter (in enhanced
mode) and primitive value converters, it would indeed be secure and be
guaranteed never to invoke untrusted code as it only sets state and
bypasses constructors, setters, etc. We could also include other converters
for types when we trust that deserialization would have no negative
side-effect (eg. String, ArrayList, HashMap, File, etc). If we were to just
use these converters, it would be secure.

It's only when we introduce converters that can instantiate arbitrary
classes that the problem is introduced.

Here are some recommendations I have for the XStream team (and of course,
it's easy for me to give recommendations as I dont actually have to do the
work):

1. It is the XStream teams responsibility to educate the users on the
issue. We should also reach out to downstream teams (eg Spring) to ensure
their users are educated too.

2. Audit the converters and see which of them can be deemed unsafe.
Specifically, any that allow non-trusted code paths to be executed.
Converters like ReflectionConverter in enhanced mode and primitive
converters are safe. DynamicProxyConverter is definitely not safe.

3. Create a stripped down "secure" mode for XStream that only includes only
safe converters. Converters that invoke constructors (e.g. ListConverter)
should include a whitelist to prevent untrusted subclasses.

4. Allow the ReflectionConverter to include a whitelist of classes/packages
it's allowed to handle. This prevents the case of a valid, yet dangerous,
class being deserialized into the runtime and later invoked polymorphically
by user code expecting it do something different.

5. Make XStream safe and secure by default to prevent accidental things.
Where users want the convenience of things just working everywhere (as they
do in XStream right now), they have to explicitly call a method somewhere,
which will ensure they actively make a decision to bypass safety. Basically
a shift in priorities, where safeness is valued over convenience. This of
course would result in a breaking behavioral change to XStream, so it
should warrant a major version release.

I acknowledge that many users use XStream in a safe manner where they can
trust the input, but the scenario of untrusted input is still large enough
and dangerous enough that I feel the team should shift priorities.

If steps 2-5 were implemented correctly, step 1 may become redundant.

Thoughts?

-Joe

p.s. Sorry if I throw things into disarray ;)

On Sunday, December 22, 2013, Dinis Cruz wrote:

> Hi, I wrote this blog post XStream "Remote Code Execution" exploit on
> code from "Standard way to serialize and deserialize Objects with XStream"
> article<http://blog.diniscruz.com/2013/12/xstream-remote-code-execution-exploit.html>
>  which
> is based on the research that Alvaro mentioned in his Is it possible to
> unregister the DynamicProxyConverter using the SpringOXM 
> wrapper<https://www.mail-archive.com/[email protected]/msg00602.html> 
> post
> here.
>
> As you can see on my blog, at the moment the users of XStream are not
> really aware of the problems that can occur when untrusted (i.e.
> potentially maliciously controlled) XML data is fed to XStream 
> *fromXml*function. Which is specially problematic when APIs are used that 
> wrap the
> use of XStream and hide its use.
>
> You can also read that I'm currently not 100% sure on the best way to
> mitigate this issue and what should be the advise to give developers (and
> pentesters/code-reviewers) in order for them to detect and fix their
> applications.
>
> So my question is: *What are the best practices and Security Guidance to
> use XStream safely?*
>
> Note that although the Remote-code-execution PoC that I presented is good
> for demos (and to raise the awareness of the issue), I think that in most
> real-world apps (with large codebases) there will be other massive security
> vulnerabilities if XStream is allowed to create any class currently
> available in the class path (for example in a banking app, it might be
> possible to create transactions objects and other business sensitive
> actions (the exploit will depend on the capabilities of the target
> application)).
>
> Thanks
>
> Dinis Cruz
>

Reply via email to