Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Config] Disable XML external entity loading for all versions of libxml#38564
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
stof commentedOct 14, 2020
this should be reported as a bug in the PHP issue tracker IMO, as that might invalidate the decision to deprecate the function. |
fabianbadoi commentedOct 14, 2020
I tried reporting it this morning, the bug tracker was down :( |
fabianbadoi commentedOct 14, 2020
We have a bug report:https://bugs.php.net/bug.php?id=80235 |
The libxml_disable_entity_loader() function was deprecated in PHP 8.0and will trigger a deprecation notice going forward. However, externalentity loading is still *enabled*.The PR where the function was deprecated,php/php-src#5867,claims that XXE loading is diabled by default since libxml 2.9.0.However, this was not the case in at least the following systems:- the php:7.4-fpm-alpine docker image- the php:8.0-rc-cli-alpine- the default php-7.4 package from the Ubuntu 20.04 package repositoriesAll of these installations report LIBXML_VERSION > 20900 and are stillvulnerable to XXE attacks unless libxml_disable_entity_loader() is used.Calling libxml_disable_entity_loader() before Kernel::hande() fails nowwith a XmlParsingException due to XmlFileLoader not temporarily enablingXXE loading. The external entity in question was src/Symfony/[...]/services-1.0.xsdwhich was being used to validate src/Symfony/[...]/config/web.xml.Calling libxml_disable_entity_loader() is recommended by OWASP, and isa very good security practice to globally prevent XXE attacks:https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html#phpYou can modify the xxe_schema.xsd file introduced in this commit topoint to ahttps://webhook.site/ URL instead of file:///dev/null. Thiswill demonstrate that these XXE calls will even try to make networkrequests.
fabianbadoi commentedOct 14, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I'd like some guidance on this PR:
I have nothing against chaning it here, idk if you would accept unrelated changes. |
stof commentedOct 14, 2020
well, that's precisely why I asked to report an issue to PHP. Symfony has a policy of not using deprecated APIs. So if using the deprecated API of PHP cannot be avoided, there is an issue with the fact of deprecating. |
stof commentedOct 14, 2020
this link is private |
fabianbadoi commentedOct 14, 2020
I also think we should wait to see what the PHP devs do with this, since they might not consider it serious enough to address. Installations are still safe from the traditional XXE attacks that reads local files. |
fabianbadoi commentedOct 14, 2020
I think they set all tickets related to security to private by default |
nicolas-grekas commentedOct 14, 2020
I rebased the PR for branch 3.4 but there are some failure to fix, could you please have a look? |
nicolas-grekas commentedOct 14, 2020
(nvm the rebase for 3.4, that's on another PR) |
jderusse commentedOct 15, 2020
The PHP bug has been closed. Calling |
fabianbadoi commentedOct 16, 2020
Yes, closing. |
Uh oh!
There was an error while loading.Please reload this page.
The libxml_disable_entity_loader() function was deprecated in PHP 8.0 and will trigger a deprecation notice going forward. However,external entity loading is still enabled.
The PR where the function was deprecated,php/php-src#5867, claims that XXE loading is diabled by default since libxml 2.9.0.
However, this was not the case in at least the following systems:
All of these installations report LIBXML_VERSION > 20900 and are still vulnerable to XXE attacks unless libxml_disable_entity_loader() is used.
Calling libxml_disable_entity_loader() before Kernel::hande() fails now with a XmlParsingException due to XmlFileLoader not temporarily enabling XXE loading. The external entity in question was src/Symfony/[...]/services-1.0.xsd which was being used to validate src/Symfony/[...]/config/web.xml.
Calling libxml_disable_entity_loader() is recommended by OWASP, and is a very good security practice to globally prevent XXE attacks:
https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html#php
You can modify the xxe_schema.xsd file introduced in this commit to point to ahttps://webhook.site/ URL instead of file:///dev/null. This will demonstrate that these XXE calls will even try to make network requests.
I don't like where I placed the test I added. I chose XmlFileLoaderTest because that XmlFileLoader was causing Symfony to crash in my case. I think it's a good test to have, though, as it will prevent regressions on this. Perhasp you can suggest a better place.