Hi Alvaro,

Alvaro wrote:

> Lets say you have a Contact Interface and a ContactImpl class.
> Contact c = (Contact) xstream.fromXML(xml);
> and xml is:
>       <dynamic-proxy>
>       <interface>org.company.model.Contact</interface>
>       <handler class="java.beans.EventHandler">
>           <target class="java.lang.ProcessBuilder">
>               <command>
>                    <string>calc.exe</string>
>               </command>
>           </target>
>           <action>start</action>
>       </handler>
>       </dynamic-proxy>
> Then as soon as the code calls any method on the Contact instace, the
> payload gets executed (eg: contact.getFirstName() )

Ah, that one. Yes, I have seen it before, but I do not blame the dynamic 
proxy by default. It's even questionable if this is a special problem with 
XStream. While this *is* a security hole, it is made possible by 
EventHandler's create method:

=============== %< ==============
public class Main {
    public static void main(String[] args) {
        Set<Comparable> set = new TreeSet<Comparable>();
        set.add(EventHandler.create(Comparable.class, new 
ProcessBuilder("gvim", "/home/joehni/tmp/ps-aux.txt"), "start"));
=============== %< ==============

While this is demonstrates the problem in pure Java, you will have the same 
security hole in any *scripted* environment that can call the JVM. Like 
XStream you can demonstrate this also with the Java ScriptEngine or quite 
any Dependency Injection container that is configured from the outside. It 
is a bit like SQL injection - if you accept input you have to ensure that it 
cannot be abused.

However, it is unfortunate that the JRE itself already provides such an easy 
way to redirect a method call to an arbitrary method of a target, but it is 
explicitly documented: 
java.lang.Object, java.lang.String)>

There's no general solution for every use case though, but it is quite easy 
to write a converter for an EventHandler or a ProcessBuilder that will throw 
an Exception at deserialization time. But it still depends on the use case. 
XStream cannot say if the deserialization of such an object is wrong in 
general or if it is a valid element of the object graph. In the latter case 
an implementation of a EventHandlerConverter might only throw an exception 
if the action name does not match a method of the proxied interface. 
Nevertheless, there is a reason why this functionality made it into the JRE 
and an object graph with GUI objects might contain such an element with much 
higher probability than expected.

Therefore I was never sure if XStream should register such a converter by 
default, but it is really frightening to have such an easy possibility for 

Actually you have 3 parts in the plain JDK itself that makes this possible:
- the dynamic proxy that can be used to insert a matching replacement
- the EventHandler that translates every call into an arbitrary action
- the ProcessBuilder that supports the call of arbitrary applications

And XStream provides with the reflection-based converters the 4th part by 
deserializing arbitrary objects.

> SpringOXM has a wrapper for XStream
> (org.springframework.oxm.xstream.XStreamMarshaller) that enables the
> unmarshalling of objects from XML format.
> This SpringOXM module is used by SpringMVC when building RESTFul APIs.
> My concern is that an attacker can sends a malicious crafted XML that
> results in remote code execution in the case that the server is expecting
> an object that implements an interface.
> I would love to be able to disable the DynamicProxyconverter in simple
> fashion and expose that method to the SpringgOXM wrapper so it can be
> safely used for RESTFul APIs.

As said, I have no knowledge about the configuration possibility of XStream 
within SpringOXM.

However, in case of untrusted input, you're only on the safe side, if you 
prevent XStream from using the reflection converters although it is enough 
to break the action in your example by suppressing one of the 4 involved 
parts. But as long as the reflection-based converters work, an attacker mind 
find other ways.

This implies that you use dedicated converters for all of your objects (you 
might use as a more generic alternative the JavaBeanConverter) and register 
a converter that claims to convert any type with low priority, but that will 
throw an exception in unmarshal. Since the reflection-based converters have 
even lower priority, they're never called.


To unsubscribe from this list, please visit:


Reply via email to