Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lead to RCE when unmarshal xml data with XStream #454

Open
idealzh opened this issue Sep 28, 2018 · 7 comments
Open

Lead to RCE when unmarshal xml data with XStream #454

idealzh opened this issue Sep 28, 2018 · 7 comments

Comments

@idealzh
Copy link

idealzh commented Sep 28, 2018

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.

@decebals
Copy link
Member

@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.

@idealzh
Copy link
Author

idealzh commented Sep 29, 2018

@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.

@decebals
Copy link
Member

@idealzh
If you have questions about Pippo, we are here to respond you.

@mhagnumdw
Copy link
Member

This issue may have a point in common with issue #458.

@mhagnumdw mhagnumdw mentioned this issue Oct 26, 2018
@rygel
Copy link
Member

rygel commented Oct 26, 2018

@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?

@mhagnumdw
Copy link
Member

@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.

@rygel
Copy link
Member

rygel commented Oct 26, 2018

@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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants