From 5bf4f92e86c34690d71e8f94350ec975909a435b Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Mon, 27 Aug 2012 19:17:44 +0200 Subject: [PATCH] fixed XML decoding attack vector through external entities --- .../DependencyInjection/Loader/XmlFileLoader.php | 6 +++++- src/Symfony/Component/DomCrawler/Crawler.php | 12 +++++++++++- .../Component/Routing/Loader/XmlFileLoader.php | 6 +++++- .../Component/Translation/Loader/XliffFileLoader.php | 8 +++++++- .../Validator/Mapping/Loader/XmlFileLoader.php | 9 ++++++++- 5 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php index 8a42e6662d..93880f2645 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php @@ -212,16 +212,20 @@ class XmlFileLoader extends FileLoader private function parseFile($file) { $internalErrors = libxml_use_internal_errors(true); + $disableEntities = libxml_disable_entity_loader(true); libxml_clear_errors(); $dom = new \DOMDocument(); $dom->validateOnParse = true; - if (!$dom->load($file, LIBXML_NONET | (defined('LIBXML_COMPACT') ? LIBXML_COMPACT : 0))) { + if (!$dom->loadXML(file_get_contents($file), LIBXML_NONET | (defined('LIBXML_COMPACT') ? LIBXML_COMPACT : 0))) { + libxml_disable_entity_loader($disableEntities); + throw new \InvalidArgumentException(implode("\n", $this->getXmlErrors($internalErrors))); } $dom->normalizeDocument(); libxml_use_internal_errors($internalErrors); + libxml_disable_entity_loader($disableEntities); foreach ($dom->childNodes as $child) { if ($child->nodeType === XML_DOCUMENT_TYPE_NODE) { diff --git a/src/Symfony/Component/DomCrawler/Crawler.php b/src/Symfony/Component/DomCrawler/Crawler.php index 02e07c8e6f..f7edbd7b64 100644 --- a/src/Symfony/Component/DomCrawler/Crawler.php +++ b/src/Symfony/Component/DomCrawler/Crawler.php @@ -119,10 +119,15 @@ class Crawler extends \SplObjectStorage */ public function addHtmlContent($content, $charset = 'UTF-8') { + $disableEntities = libxml_disable_entity_loader(true); + $dom = new \DOMDocument('1.0', $charset); $dom->validateOnParse = true; @$dom->loadHTML($content); + + libxml_disable_entity_loader($disableEntities); + $this->addDocument($dom); $base = $this->filter('base')->extract(array('href')); @@ -142,11 +147,16 @@ class Crawler extends \SplObjectStorage */ public function addXmlContent($content, $charset = 'UTF-8') { + $disableEntities = libxml_disable_entity_loader(true); + $dom = new \DOMDocument('1.0', $charset); $dom->validateOnParse = true; // remove the default namespace to make XPath expressions simpler - @$dom->loadXML(str_replace('xmlns', 'ns', $content)); + @$dom->loadXML(str_replace('xmlns', 'ns', $content), LIBXML_NONET); + + libxml_disable_entity_loader($disableEntities); + $this->addDocument($dom); } diff --git a/src/Symfony/Component/Routing/Loader/XmlFileLoader.php b/src/Symfony/Component/Routing/Loader/XmlFileLoader.php index bfe6d62f00..5dad9db3fa 100644 --- a/src/Symfony/Component/Routing/Loader/XmlFileLoader.php +++ b/src/Symfony/Component/Routing/Loader/XmlFileLoader.php @@ -151,16 +151,20 @@ class XmlFileLoader extends FileLoader protected function loadFile($file) { $internalErrors = libxml_use_internal_errors(true); + $disableEntities = libxml_disable_entity_loader(true); libxml_clear_errors(); $dom = new \DOMDocument(); $dom->validateOnParse = true; - if (!$dom->load($file, LIBXML_NONET | (defined('LIBXML_COMPACT') ? LIBXML_COMPACT : 0))) { + if (!$dom->loadXML(file_get_contents($file), LIBXML_NONET | (defined('LIBXML_COMPACT') ? LIBXML_COMPACT : 0))) { + libxml_disable_entity_loader($disableEntities); + throw new \InvalidArgumentException(implode("\n", $this->getXmlErrors($internalErrors))); } $dom->normalizeDocument(); libxml_use_internal_errors($internalErrors); + libxml_disable_entity_loader($disableEntities); foreach ($dom->childNodes as $child) { if ($child->nodeType === XML_DOCUMENT_TYPE_NODE) { diff --git a/src/Symfony/Component/Translation/Loader/XliffFileLoader.php b/src/Symfony/Component/Translation/Loader/XliffFileLoader.php index fdc615547d..eb222fc552 100644 --- a/src/Symfony/Component/Translation/Loader/XliffFileLoader.php +++ b/src/Symfony/Component/Translation/Loader/XliffFileLoader.php @@ -56,14 +56,19 @@ class XliffFileLoader implements LoaderInterface private function parseFile($file) { $internalErrors = libxml_use_internal_errors(true); + $disableEntities = libxml_disable_entity_loader(true); libxml_clear_errors(); $dom = new \DOMDocument(); $dom->validateOnParse = true; - if (!@$dom->load($file, LIBXML_NONET | (defined('LIBXML_COMPACT') ? LIBXML_COMPACT : 0))) { + if (!@$dom->loadXML(file_get_contents($file), LIBXML_NONET | (defined('LIBXML_COMPACT') ? LIBXML_COMPACT : 0))) { + libxml_disable_entity_loader($disableEntities); + throw new \RuntimeException(implode("\n", $this->getXmlErrors($internalErrors))); } + libxml_disable_entity_loader($disableEntities); + foreach ($dom->childNodes as $child) { if ($child->nodeType === XML_DOCUMENT_TYPE_NODE) { libxml_use_internal_errors($internalErrors); @@ -90,6 +95,7 @@ class XliffFileLoader implements LoaderInterface if (!@$dom->schemaValidateSource($source)) { throw new \RuntimeException(implode("\n", $this->getXmlErrors($internalErrors))); } + $dom->normalizeDocument(); libxml_use_internal_errors($internalErrors); diff --git a/src/Symfony/Component/Validator/Mapping/Loader/XmlFileLoader.php b/src/Symfony/Component/Validator/Mapping/Loader/XmlFileLoader.php index a45ab5a1ca..e44ebfe136 100644 --- a/src/Symfony/Component/Validator/Mapping/Loader/XmlFileLoader.php +++ b/src/Symfony/Component/Validator/Mapping/Loader/XmlFileLoader.php @@ -181,16 +181,23 @@ class XmlFileLoader extends FileLoader protected function parseFile($file) { $internalErrors = libxml_use_internal_errors(true); + $disableEntities = libxml_disable_entity_loader(true); libxml_clear_errors(); $dom = new \DOMDocument(); $dom->validateOnParse = true; - if (!$dom->load($file, LIBXML_NONET | (defined('LIBXML_COMPACT') ? LIBXML_COMPACT : 0))) { + if (!$dom->loadXML(file_get_contents($file), LIBXML_NONET | (defined('LIBXML_COMPACT') ? LIBXML_COMPACT : 0))) { + libxml_disable_entity_loader($disableEntities); + throw new MappingException(implode("\n", $this->getXmlErrors($internalErrors))); } + + libxml_disable_entity_loader($disableEntities); + if (!$dom->schemaValidate(__DIR__.'/schema/dic/constraint-mapping/constraint-mapping-1.0.xsd')) { throw new MappingException(implode("\n", $this->getXmlErrors($internalErrors))); } + $dom->normalizeDocument(); libxml_use_internal_errors($internalErrors);