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("foo"); 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: <http://docs.oracle.com/javase/7/docs/api/java/beans/EventHandler.html#create(java.lang.Class, 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 abuse. 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. Regards, Jörg --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email