Skip to content

[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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

[Config] Disable XML external entity loading for all versions of libxml #38564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 5 additions & 9 deletions src/Symfony/Component/Config/Tests/Util/XmlUtilsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,9 +199,7 @@ public function testLoadEmptyXmlFile()
// test for issue https://github.com/symfony/symfony/issues/9731
public function testLoadWrongEmptyXMLWithErrorHandler()
{
if (\LIBXML_VERSION < 20900) {
$originalDisableEntities = libxml_disable_entity_loader(false);
}
$originalDisableEntities = libxml_disable_entity_loader(false);
$errorReporting = error_reporting(-1);

set_error_handler(function ($errno, $errstr) {
Expand All @@ -221,13 +219,11 @@ public function testLoadWrongEmptyXMLWithErrorHandler()
error_reporting($errorReporting);
}

if (\LIBXML_VERSION < 20900) {
$disableEntities = libxml_disable_entity_loader(true);
libxml_disable_entity_loader($disableEntities);
$disableEntities = libxml_disable_entity_loader(true);
libxml_disable_entity_loader($disableEntities);

libxml_disable_entity_loader($originalDisableEntities);
$this->assertFalse($disableEntities);
}
libxml_disable_entity_loader($originalDisableEntities);
$this->assertFalse($disableEntities);

// should not throw an exception
XmlUtils::loadFile(__DIR__.'/../Fixtures/Util/valid.xml', __DIR__.'/../Fixtures/Util/schema.xsd');
Expand Down
12 changes: 3 additions & 9 deletions src/Symfony/Component/Config/Util/XmlUtils.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,27 +51,21 @@ public static function parse($content, $schemaOrCallable = null)
}

$internalErrors = libxml_use_internal_errors(true);
if (\LIBXML_VERSION < 20900) {
$disableEntities = libxml_disable_entity_loader(true);
}
$disableEntities = libxml_disable_entity_loader(true);
libxml_clear_errors();

$dom = new \DOMDocument();
$dom->validateOnParse = true;
if (!$dom->loadXML($content, \LIBXML_NONET | (\defined('LIBXML_COMPACT') ? \LIBXML_COMPACT : 0))) {
if (\LIBXML_VERSION < 20900) {
libxml_disable_entity_loader($disableEntities);
}
libxml_disable_entity_loader($disableEntities);

throw new XmlParsingException(implode("\n", static::getXmlErrors($internalErrors)));
}

$dom->normalizeDocument();

libxml_use_internal_errors($internalErrors);
if (\LIBXML_VERSION < 20900) {
libxml_disable_entity_loader($disableEntities);
}
libxml_disable_entity_loader($disableEntities);

foreach ($dom->childNodes as $child) {
if (\XML_DOCUMENT_TYPE_NODE === $child->nodeType) {
Expand Down
10 changes: 3 additions & 7 deletions src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,13 +620,9 @@ public function validateSchema(\DOMDocument $dom)
EOF
;

if (\LIBXML_VERSION < 20900) {
$disableEntities = libxml_disable_entity_loader(false);
$valid = @$dom->schemaValidateSource($source);
libxml_disable_entity_loader($disableEntities);
} else {
$valid = @$dom->schemaValidateSource($source);
}
$disableEntities = libxml_disable_entity_loader(false);
$valid = @$dom->schemaValidateSource($source);
libxml_disable_entity_loader($disableEntities);

foreach ($tmpfiles as $tmpfile) {
@unlink($tmpfile);
Expand Down
10 changes: 10 additions & 0 deletions src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/xxe_schema.xsd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0" encoding="utf-8" ?>
<xsd:schema xmlns="http://symfony.com/schema"
xmlns:xsd="http://www.w3.org/2001/XMLSchema"
targetNamespace="http://symfony.com/schema"
elementFormDefault="qualified">

<xsd:import namespace="http://www.w3.org/XML/1998/namespace"/>
<xsd:import namespace="http://symfony.com/schema/dic/services" schemaLocation="file:///dev/null" />

</xsd:schema>
31 changes: 25 additions & 6 deletions src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,21 +95,40 @@ public function testParseFile()

public function testLoadWithExternalEntitiesDisabled()
{
if (\LIBXML_VERSION < 20900) {
$disableEntities = libxml_disable_entity_loader(true);
}
$disableEntities = libxml_disable_entity_loader(true);

$containerBuilder = new ContainerBuilder();
$loader = new XmlFileLoader($containerBuilder, new FileLocator(self::$fixturesPath.'/xml'));
$loader->load('services2.xml');

if (\LIBXML_VERSION < 20900) {
libxml_disable_entity_loader($disableEntities);
}
libxml_disable_entity_loader($disableEntities);

$this->assertGreaterThan(0, $containerBuilder->getParameterBag()->all(), 'Parameters can be read from the config file.');
}

/**
* Tests to see whether or not calling libxml_disable_entity_loader() is still needed.
*
* The xml/xxe_schema.xsd highlights simulates a potential potential XXE attack by referencing an external file.
* Depending on the outcome of the schemaValidate() call, we can determine if calling libxml_disable_entity_loader()
* is required on that specific platform.
*
* External schemas are used internally in Symfony with local files only, and several classes enable external
* entity loading temporarily.
*
* @see https://owasp.org/www-community/vulnerabilities/XML_External_Entity_(XXE)_Processing
*/
public function testDisableEntityLoaderNeeded()
{
$this->expectWarning();
$this->expectWarningMessage('Document is empty');
// if loading external entities fails the warning message would contain "failed to load external entity"

$dom = new \DOMDocument();

$dom->schemaValidate(self::$fixturesPath.'/xml/xxe_schema.xsd');
}

public function testLoadParameters()
{
$container = new ContainerBuilder();
Expand Down
16 changes: 4 additions & 12 deletions src/Symfony/Component/DomCrawler/Crawler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,7 @@ public function addContent($content, $type = null)
public function addHtmlContent($content, $charset = 'UTF-8')
{
$internalErrors = libxml_use_internal_errors(true);
if (\LIBXML_VERSION < 20900) {
$disableEntities = libxml_disable_entity_loader(true);
}
$disableEntities = libxml_disable_entity_loader(true);

$dom = new \DOMDocument('1.0', $charset);
$dom->validateOnParse = true;
Expand All @@ -205,9 +203,7 @@ public function addHtmlContent($content, $charset = 'UTF-8')
}

libxml_use_internal_errors($internalErrors);
if (\LIBXML_VERSION < 20900) {
libxml_disable_entity_loader($disableEntities);
}
libxml_disable_entity_loader($disableEntities);

$this->addDocument($dom);

Expand Down Expand Up @@ -250,9 +246,7 @@ public function addXmlContent($content, $charset = 'UTF-8', $options = \LIBXML_N
}

$internalErrors = libxml_use_internal_errors(true);
if (\LIBXML_VERSION < 20900) {
$disableEntities = libxml_disable_entity_loader(true);
}
$disableEntities = libxml_disable_entity_loader(true);

$dom = new \DOMDocument('1.0', $charset);
$dom->validateOnParse = true;
Expand All @@ -262,9 +256,7 @@ public function addXmlContent($content, $charset = 'UTF-8', $options = \LIBXML_N
}

libxml_use_internal_errors($internalErrors);
if (\LIBXML_VERSION < 20900) {
libxml_disable_entity_loader($disableEntities);
}
libxml_disable_entity_loader($disableEntities);

$this->addDocument($dom);

Expand Down
8 changes: 2 additions & 6 deletions src/Symfony/Component/Serializer/Encoder/XmlEncoder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,18 +83,14 @@ public function decode($data, $format, array $context = [])
}

$internalErrors = libxml_use_internal_errors(true);
if (\LIBXML_VERSION < 20900) {
$disableEntities = libxml_disable_entity_loader(true);
}
$disableEntities = libxml_disable_entity_loader(true);
libxml_clear_errors();

$dom = new \DOMDocument();
$dom->loadXML($data, $this->loadOptions);

libxml_use_internal_errors($internalErrors);
if (\LIBXML_VERSION < 20900) {
libxml_disable_entity_loader($disableEntities);
}
libxml_disable_entity_loader($disableEntities);

if ($error = libxml_get_last_error()) {
libxml_clear_errors();
Expand Down
10 changes: 3 additions & 7 deletions src/Symfony/Component/Translation/Loader/XliffFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,13 +189,9 @@ private function validateSchema($file, \DOMDocument $dom, $schema)
{
$internalErrors = libxml_use_internal_errors(true);

if (\LIBXML_VERSION < 20900) {
$disableEntities = libxml_disable_entity_loader(false);
$isValid = @$dom->schemaValidateSource($schema);
libxml_disable_entity_loader($disableEntities);
} else {
$isValid = @$dom->schemaValidateSource($schema);
}
$disableEntities = libxml_disable_entity_loader(false);
$isValid = @$dom->schemaValidateSource($schema);
libxml_disable_entity_loader($disableEntities);

if (!$isValid) {
throw new InvalidResourceException(sprintf('Invalid resource provided: "%s"; Errors: ', $file).implode("\n", $this->getXmlErrors($internalErrors)));
Expand Down
8 changes: 2 additions & 6 deletions src/Symfony/Component/Translation/Tests/Loader/XliffFileLoaderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,13 @@ public function testLoadWithInternalErrorsEnabled()

public function testLoadWithExternalEntitiesDisabled()
{
if (\LIBXML_VERSION < 20900) {
$disableEntities = libxml_disable_entity_loader(true);
}
$disableEntities = libxml_disable_entity_loader(true);

$loader = new XliffFileLoader();
$resource = __DIR__.'/../fixtures/resources.xlf';
$catalogue = $loader->load($resource, 'en', 'domain1');

if (\LIBXML_VERSION < 20900) {
libxml_disable_entity_loader($disableEntities);
}
libxml_disable_entity_loader($disableEntities);

$this->assertEquals('en', $catalogue->getLocale());
$this->assertEquals([new FileResource($resource)], $catalogue->getResources());
Expand Down