pippo-java
Lead to RCE when unmarshal xml data with XStream
The XstreamEngine unmarshal xml data based on XStream, but it doesn't check the data; when the data contains malicious types, then it may leads to remote code execution; The Struts2 framework once had the same issue(https://cwiki.apache.org/confluence/display/WW/S2-052); Using the following code snippet to convert the malicious xml data:
POST("/xml", routeContext -> {
String xmlData = routeContext.getRequest().getBody();
XstreamEngine engine = new XstreamEngine();
engine.fromString(xmlData, String.class);
});
The malicious xml data is as follows：
payload.txt
The tool marshalsec can help to generate more kinds of payload including the one above.
To mitigate the vulnerability, since version 1.4.7, XStream provides developers with some APIs (such as addPermission,denyPermission, allowTypes,denyTypes) to restrict the types be unmarshalled.
Here we could fix the issue refer to the patch of Struts2.
@idealzh Thanks! I understand the problem. Do you have a solution, or a recommendation for other users that use pippo-xtream? Do you think that we can add a protection in Pippo? If yes, please submit a PR.
@decebals Hi,decebals! Please give me some time to know about the mechanism of pippo better,and implement the protection code;I will submit a pr later.
@mhagnumdw Thank you for explaining the issue better in the comments of #470 . I understand now that both #458 and #454 are related. I am now just wondering this issue is fixed with #470 or if there is more work needed?
@rygel , thank you, I hope to help when possible!
From what I understand this issue has not been resolved. But, looking quickly, the method XstreamEngine.xstream() can make use of the method allowTypes from XStream. Other security-related methods are listed here: http://x-stream.github.io/security.html#framework
Pippo supports several other frameworks, see here. If any of them need this adjustment I do not know, we would have to go out investigating...
I believe the case of that issue can be resolved. Further on, if needed, the other serialization frameworks can be covered with this solution.
@mhagnumdw Thank you for your support!
I see. I think it would be good if we only ave one mechanism to whitelist classes for deserializing for all frameworks used by Pippo. Maybe that is the reason why @decebals suggested to move the WhitelistObjectInputStream.java to core?