ngallagher / simplexml

Simple XML
Apache License 2.0
97 stars 37 forks source link

XXE vulnerability in SimpleXML #18

Open OneSourceCat opened 7 years ago

OneSourceCat commented 7 years ago

Hi, there. Recently, I learned about SimpleXML and tried my luck to find some bugs. Here is what I found. As you know, SimpleXML can serialize and deserialize XML document. So I tested for these functions and finally I found these can lead to an XXE vulnerability. First, I tried to inject payloads into the attributes. However, SimpleXML cannot resolve external entity in attribute of the element.

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE user [<!ENTITY internal SYSTEM 'file:///c:/windows/win.ini'>]>  
<example index="&internal;">
    <text>Example message</text>
</example>

It will raise an exception when resolve the XML document above because SimpleXML cannot resolve external entity in attributes. However, external entity can be used at elements' text nodes:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE user [<!ENTITY internal SYSTEM 'file:///c:/windows/win.ini'>]>  
<example index="123">
    <!-- SimpleXML didn't forbid external entity in xml elements-->
    <text>Example message:&internal;</text>
</example>

When deserialize this document above, we can retrieve the content of win.ini in C disk. Also, we can use http, gopher, dict protocol in XXE attack. More details in PoC.java(test version is 2.7.1). Besides, Retrofit is also affected by this vulnerability because of the usage of SimpleXML inside. xxe

PoC.txt

To fix it, you can limit the resolving of external entities in XML document. Let me know if you have problem to reproduce this issue. Thanks!

OneSourceCat commented 7 years ago

Any comment?

ngallagher commented 7 years ago

Hi, So this has nothing to do with SimpleXML, it has to do with the underlying library. You can choose to use XPP, StAX, or DOM. You can configure these however you like, or supply an implementation other than the defaults provided by the platform you run on. Regards,Niall

  From: John Saigle <notifications@github.com>

To: ngallagher/simplexml simplexml@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Sent: Wednesday, 31 May 2017, 20:27 Subject: Re: [ngallagher/simplexml] XXE vulnerability in SimpleXML (#18)

@OneSourceCat is this still potentially exploitable? have you received any contact?This is relevant to a project I'm working on and anything you could tell me would be helpful.— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

OneSourceCat commented 7 years ago

This morning I received the CVE ID about this, otherwise I even forgot that I had submmited this issue. SimpleXML has to ban resolving the external entities by default. The developers don't have duty and abilities to handle such security problem. Ban resolving the exteral entities is a good way to prevent this vulnerability.

ngallagher commented 7 years ago

SimpleXML does not parse XML... it is XMPP, DOM, or StAX. You can control this. I will not address an issue that does not exist.

On Thursday, August 24, 2017, 4:23:22 AM GMT+1, RuiChong notifications@github.com wrote:

This morning I received the CVE ID about this, otherwise I even forgot that I had submmited this issue. SimpleXML has to ban resolving the external entities by default. The developers don't have duty and abilities to handle such security problem. Ban resolving the exteral entities is a good way to prevent this vulnerability.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub, or mute the thread.

Kisty commented 7 years ago

@ngallagher @OneSourceCat can you explain how this can be controlled by the user? Are you saying that you can configure the XML parser (XMPP, DOM or StAX) externally from SimpleXML? If so, how?

danajanezic commented 6 years ago

@ngallagher @OneSourceCat I too would like an explanation. I have not been able to put the magic words into google to return results that will show me how to do this.

CMYanko commented 5 years ago

Would love to see some response on this. I understand it may not be an issue with SimpleXML but we need some guidance on how to mitigate. Is it one of the project dependencies that can be updated?

Are we talking about creating a 'filter' in StAX? .or is the issue in the DocumentProvider.class

Simulant87 commented 5 years ago

You should migrate from group: 'org.simpleframework', name: 'simple-xml', version: "2.7" to group: 'org.restlet.lib', name: 'org.simpleframework.simple-xml', version: "2.7.3"

dweiss commented 5 years ago

@ngallagher I think it'd be convenient to make StreamProvider public and accepting a factory in its constructor. Then you can pass a preconfigured factory (disabling entity resolution) and use it for reading the InputNode which in turn can be passed to the Persister. These are minimal API changes and allow for xml factory configuration (used locally) to happen outside of simple-xml. These also don't require any global changes which are often impossible or inconvenient.

dweiss commented 5 years ago

I created a fork and implemented the changes I mentioned above since simple-xml makes so many people so nervous because of CVEs. https://github.com/dweiss/simplexml

The "default" Provider configuration disables entity expansion in that fork. You can still create a version that works with everything the underlying XML parser is configured with.

LarryKlugerDS commented 4 years ago

Thank you @dweiss !!

dweiss commented 4 years ago

Welcome!

dawinter commented 4 years ago

Is there any plan to build and provide a new version with the fix for CVE-2017-1000190?

artem-smotrakov commented 4 years ago

The solution developed by @dweiss looks correct to me:

https://github.com/dweiss/simplexml

Only one comment. Although the tests show that XXE and Billion laughs don't work anymore, to be on the safe side, it may also enable the FEATURE_SECURE_PROCESSING option

https://docs.oracle.com/javase/7/docs/api/javax/xml/XMLConstants.html#FEATURE_SECURE_PROCESSING

@ngallagher Would it be possible to merge the patch and release a new version of simple-xml?

Please let me know if you need help.

Sachpat commented 4 years ago

@ngallagher Yes if possible, please try to merge the fix and provide a new version of sample-xml which fixes CVE-2017-1000190.

nngo commented 3 years ago

FYI: you can address this issue by using the forked com.carrotsearch.thirdparty:simple-xml-safe:2.7.1 (https://github.com/carrotsearch/simplexml-safe) that has this fixed (that is what the MINIO folks did in their minio-java 7.1.0 release)

dileep121923 commented 2 years ago

@ngallagher Yes if possible, please try to merge the fix and provide a new version of sample-xml which fixes CVE-2017-1000190.

Does this fix available online? I couldn't find the fixed version in maven or any other repositories

avithapa commented 4 months ago

When will be a fix available for this?

prabhakar-mahendrakar commented 3 months ago

We see a vulnerability "CVE-2017-1000190" detected with simple xml 2.7.1 version. Please provide the recommendation you guys have to avoid this security issue? Is there any plan to build and provide a new version with the fix for CVE-2017-1000190?