Disabling External Entities in libxml By Default

Hi,

-----Original Message-----
From: Pierre Joye [mailto:pierre.php@gmail.com]
Sent: Wednesday, July 29, 2015 11:01 PM
To: Anthony Ferrara ircmaxell@gmail.com
Cc: PHP internals internals@lists.php.net
Subject: Re: [PHP-DEV] Disabling External Entities in libxml By Default

All,

I wanted to float an idea by you for PHP 7 (or 7.1 depending on the
RM's feedback).

Currently, PHP by default is vulnerable to XXE attacks:
https://www.owasp.org/index.php/XML_External_Entity_(XXE)_Processing

To bypass this, you need to turn off external entity loading:

libxml_disable_entity_loader(true);

What I'm proposing is to disable entity loading by default. That way
it requires developers to opt-in to actually load external entities.

Thoughts?

I am for it, for 7.0 or 8.0.

We discussed it during the last related flaw and decided not to do it for BC
reasons (whatever it means in this case).

This problem went off our radar, so yes, we should do it in 7.0. Changing default
in minor versions always create more troubles.

To note were that the libxml-2.9.2 in Windows builds already contains patches mentioned in https://www.debian.org/security/2013/dsa-2652 , see https://github.com/winlibs/libxml2/commit/727e357fb21b95d5c315518bdac99a70a6d15ff8 ... Most of the distributions should already have these patches. Probably we should check whether disabling it in PHP were unnecessary, but if it's not - ofc 7.0 should be the target at least.

Regards

Anatol

Hi!

Currently, PHP by default is vulnerable to XXE attacks:
https://www.owasp.org/index.php/XML_External_Entity_(XXE)_Processing

To bypass this, you need to turn off external entity loading:

libxml_disable_entity_loader(true);
AFAIR right now, due to how it is implemented, this blocks loading XML
content from files with something like XMLReader::open() - due to the
use of the same code path by both. It may have changes since last time I
looked, but it definitely was a major reason why default stayed that
way. What people did is something like that:

libxml_disable_entity_loader( false );
$reader->open( $filename );
libxml_disable_entity_loader( true );

I imagine we could do better. But we need to be careful - if we just set
it as disabled, we could break a lot of unsuspecting apps that do
nothing more that reading XML files.

It hasn't changed since then. I've gone back and forth on this in the
past. I've looked at both allowing the initial file to be read as well
as potentially adding support for file resources. The latter requires
app changes anyways so no big advantage there, tho would get the job done.

I'm still going back and forth on it as a few things come to mind.
If you are already working with a trusted document then you should
safely be able to disable the entity loader. If you aren't then wouldn't
you want to do some sort of checking (especially if you dont have an XML
gateway fronting the system) for other malicious things before even
opening the document regardless if it has external entities or not. The
intial doc is really an external at that point. In our systems the
entity loader is disabled right off the bat so if any developer needs to
work with xml anywhere else in the system, they have to explicitly
enable it, load and then disable it. Requires a conscious effort on
their part as well as makes it easier to audit their usage of it and the
trustworthiness of their data.

imo it might be better to leave the entity loader function as is and
introduce a new function which can be enabled by default yet allow for
the initial data to be read yet not allow any external loading from
there. This way you are not going to have to relax the security aspect
of the current function (where the current function always overrides the
ability to read the initial data if the entity loader is disabled).

One thing to be careful of tho is the breakage of unsuspecting apps. For
anyone who relies on external DTD, schemas, xslt includes, etc.. even
within their own environment, if they are not currently using the entity
loader function now, they are certain to break.

Whichever direction the consensus is for this I'm more than willing to
help implement.

Anyways just my 2 cents.

Rob

Jake,

Hello

Disabling this will (at least for me) cause SOAP related stuff to stop
working as it was expected to work before!

The problem here is that imagine the following:

<?xml version="1.0" encoding="UTF-8"?>

and then evil1.dtd:
<?xml version="1.0" encoding="UTF-8"?>

%all;

The contents of /etc/passwd would be sent as the url prameter to
http://example.com/content.

That works for any file that the server has access to.

And if you have the expect extension installed it could be MUCH worse.
You could use the URL "expect://rm -RF *" and boom goes the dynamite.

So yeah, while I completely get that some things like SOAP require
external entities, they are also exceedingly dangerous. And only to
be used with extremely trusted endpoints.

<?php
$wsdl = "https://www.some.tld/soap.php?wsdl";;

$soap = SoapServer($wsdl, array(....));

wsdl:

<?xml version="1.0" encoding="utf-8"?>
<wsdl:definitions xmlns:http="http://schemas.xmlsoap.org/wsdl/http/";
xmlns:soap="http://schemas.xmlsoap.org/wsdl/soap/";
xmlns:s="http://www.w3.org/2001/XMLSchema";

xmlns:soapenc="http://schemas.xmlsoap.org/soap/encoding/";
xmlns:tm="http://microsoft.com/wsdl/mime/textMatching/";
xmlns:mime="http://schemas.xmlsoap.org/wsdl/mime/";
xmlns:wsdl="http://schemas.xmlsoap.org/wsdl/";
xmlns:tns="http://www.some.tld/soap/muppet/user/1.0/";

targetNamespace="http://www.some.tld/soap/muppet/user/1.0/">;
wsdl:types

    <s:schema

targetNamespace="http://www.some.tld/soap/muppet/user/1.0/";
xmlns:tns="http://www.some.tld/soap/muppet/user/1.0/";
elementFormDefault="qualified">

...

It fails with

error to read external entity, failed while parsing external entity
'http://www.some.tld/muppet.php?wsdl'

I know that you want it to work, but this is actually a great place to
fail, because you're loading a trusted resource over HTTP. Meaning
that an attacker could MITM and inject malicous XML into the response,
and own your server without even needing to own the endpoint.

..

I dont know if i get this error correct but to me it looks like PHP on "www
frontend" refuse to read WSDL/SOAP/XML from "www backend" because of this...
Petty much of the SOAP idea is gone then..?

I thought SOAP was dead already.

Kidding aside, the vulnerability isn't super-well-known but also can
be extremely dangerous. Not can-be; is. So something to consider.

Anthony