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

DDC-3020: simplexml_load_file(): I/O warning: failed to load external in XmlDriver.php #3788

Closed
doctrinebot opened this issue Mar 11, 2014 · 16 comments
Assignees
Labels
Milestone

Comments

@doctrinebot
Copy link

Jira issue originally created by user rmatrono:

PHP Warning: simplexml_load_file(): I/O warning : failed to load external entity "/path-to/doctrine/entities/mappings/my_entity.dcm.xml" in /path-to/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php on line 711

PHP bug:
https://bugs.php.net/bug.php?id=62577

Possible solution:
https://github.com/owncloud/core/pull/7498/files

@doctrinebot
Copy link
Author

Comment created by @Ocramius:

You are not supposed to load external entities in mappings.

Also, mappings are not user input, therefore they are not valid XXE/XEE attack vectors.

@doctrinebot
Copy link
Author

Issue was closed with resolution "Invalid"

@doctrinebot
Copy link
Author

Comment created by @Ocramius:

Deployed a docs fix at 02daf00

@doctrinebot
Copy link
Author

Comment created by rmatrono:

this is not a bug in Doctrine, who wants a quickfix can create a custom driver and force import of XXE/XEE before drivers are used:

class MyQuickFixXmlDriver extends \Doctrine\ORM\Mapping\Driver\XmlDriver
{
    /****
     * {@inheritDoc}
     */
    public function loadMetadataForClass($className, ClassMetadata $metadata)
    {
        $loadEntities = libxml*disable_entity*loader(false);
        parent::loadMetadataForClass($className, $metadata);
        libxml*disable_entity*loader($loadEntities);
    }
} 

@layanto
Copy link

layanto commented Dec 23, 2015

This is an issue even with no external entities in orm mapping.
See https://groups.google.com/forum/m/#!topic/doctrine-user/vAb_3zcz6vU

@Ocramius
Copy link
Member

@layanto as explained above, this has nothing to do with the ORM itself.

@layanto
Copy link

layanto commented Dec 26, 2015

@Ocramius agree that the root of the problem is not Doctrine ORM, but rather it is a bug in PHP/libxml2. However, this bug has not been resolved for over 3 years and still exists in the latest versions of php5.6 and php7. Doctrine xml mapping will fail regardless of whether external entities are used or not (FosUserBundle xml mapping has no external entities) when libxml_disable_entity_loader is set to true.

If you do a Google search on "failed to load external entity symfony" or "failed to load external FosUserBundle" you will find quite a few people having issues due to Doctrine mapping failed to load xml mapping (mostly of FosUserBundle). Most people describe the problem as randomly happening, unexplained and resolved by restarting PHP (which will reset libxml_disable_entity_loader back to false).

As you described above, xml mapping is not a valid attack vector as such should be safe for Doctrine xml driver to set libxml_disable_entity_loader to false prior to loading xml mapping and then reset libxml_disable_entity_loader setting back (exactly what OwnCloud did to work around this bug https://github.com/owncloud/core/pull/7498/files).

Just wanting to say that despite being nothing to do with the ORM itself, it is a PHP/libxml bug that affects Doctrine xml mapping loader which, in my opinion, Doctrine should work around this bug. Will save people a lot of head scratching why they code randomly stopped working (as this bug is not threadsafe, another application on the same server can trigger this issue - in my case RSS app tt-rss.org).

Thanks for responding to a closed issue. At least I now know what to do to work around this issue. Just trying to save others the trouble.

http://stackoverflow.com/questions/27035821/random-error-symfonycontexterrorexception-warning-simplexml-load-file-i-o
http://www.scriptscoop.net/t/08fbd49b7bfc/php-simplexml-load-file-i-o-warning-failed-to-load-external-entity-user-bu.html

@Ocramius
Copy link
Member

Doctrine should work around this bug

Won't happen.

There is a already a workaround: enabling metadata caching

  • messing with libxml is not feasible, since it is not thread safe
  • we won't mess with libxml at all, as that is a security-sensitive area that requires horrible hacks to keep our users safe.
  • the risk is enabling external entity loading, and then having a security issue in a different script

Therefore no-go: not touching this, this has to be fixed in PHP and by PHP only, or else we just make things worse.

@Sicaine
Copy link

Sicaine commented Jul 5, 2016

@Ocramius how is enabling metadata caching a workaround?

Would have been nice to have a proper way to activate a 'workaround' because the problem apperently still exists ( i have it :( )

@Ocramius
Copy link
Member

Ocramius commented Jul 5, 2016

The workaround is to disable external entity loading completely inside your PHP installation. External entity loading should be done in a quite well armored environment anyway, as it is a really powerful and dangerous feature of XML

@apapsch
Copy link

apapsch commented Aug 14, 2017

[1] suggests this can be fixed by avoiding SimpleXML functions which operate on files, e.g. replace simplexml_load_file(...) with simplexml_load_string(file_get_contents(...)).

@Ocramius
Copy link
Member

The external entity would still be there, unless the simplexml internals completely skip external references when loading from string.

@apapsch
Copy link

apapsch commented Aug 14, 2017

That is what the Stackoverflow answer suggests. I'm applying the proposed workaround live in the vendor folder of some affected app and see if the error perists. Also I'm removing libxml_disable_entity_loader(false); that's in place as a workaround right now.

If no error pops up, then I would say the Stackoverflow answer ist right and the proposed workaround should be applied in Doctrine proper. All very voodo, of course. To gain full confidence, one would need to dig in the SimpleXml internals, I guess.

@apapsch
Copy link

apapsch commented Aug 14, 2017

So far, so good: no crash after the changes.

@apapsch
Copy link

apapsch commented Aug 18, 2017

Even OP of the PHP bug says:

I have tried with simplexml_load_string(), it works; same with new SimpleXMLElement() etc. The bug is restricted to the simplexml_load_file() function.

@Ocramius
Copy link
Member

Handled in #6633

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

5 participants