Description
From manual page: https://php.net/function.libxml-disable-entity-loader
libxml_disable_entity_loader() was deprecated in php/php-src@e0fa48f with the note:
Since PHP now requires libxml >= 2.9.0 external entity loading no longer
needs to be disabled to prevent these attacks. It is disabled by default.
The documentation for libxml_disable_entity_loader also notes:
However, as of libxml 2.9.0 entity substitution is disabled by default, so there is no need to disable the loading of external entities, unless there is the need to resolve internal entity references with LIBXML_NOENT.
However, there is no mention of the many ways it can be enabled, or how to make it safe where it has been enabled.
Previously you could call code such as:
libxml_disable_entity_loader(true);
$document = new DOMDocument();
$document->loadXML($xml, LIBXML_DTDLOAD | LIBXML_DTDATTR);
The presence of either the DTDLOAD or DTDATTR constants would normally cause the entity to be fetched, but the presence of libxml_disable_entity_loader overrdies and prevents this.
Now, after removing the deprecated call, the entity is fetched.
Whilst the honus is on the developer to understand the change to libxml, the attack vector of an XXE is not sufficiently clear and could be significantly improved in this documentation. The list of libxml constants does not adequately explain the risk of fetching the DTD, or explain that the behaviour of these flags has changed with the deprecation and removal of libxml_disable_entity_loader.
I would recommend that:
- the
libxml_disable_entity_loaderdocs be updated to note that presence of the relevantLIBXML_flags will override the default behaviour to not load the entity with advice on how to mitigate this; and - the Predefined Constants docs be updated to provide more information on the meaning of the various flags, and their impact.