From 0103f48c86ca1a15a3c6326a0a913cb5395681d1 Mon Sep 17 00:00:00 2001
From: Benni Mack <benni@typo3.org>
Date: Tue, 23 Feb 2016 11:45:30 +0100
Subject: [PATCH] [SECURITY] XML entity expansion
Remote XML entites can be loaded in places where TYPO3 expects
only local files to be fetched. All places are changed so
the option to load entities is disabled.
Resolves: #61269
Releases: master, 7.6, 6.2
Security-Commit: 736a7ef0823893047843c6a7f5e72b220bfd4697
Security-Bulletins: TYPO3-CORE-SA-2016-005, 006, 007, 008
Change-Id: I26701fc2ffb5aed7ccbd96c168aef571d012091e
Reviewed-on: https://review.typo3.org/46834
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
---
typo3/sysext/adodb/Documentation/Index.rst | 2 ++
typo3/sysext/adodb/adodb/adodb-xmlschema.inc.php | 6 ++++++
typo3/sysext/adodb/adodb/adodb-xmlschema03.inc.php | 6 ++++++
.../core/Classes/Imaging/IconProvider/SvgIconProvider.php | 3 +++
.../core/Classes/Localization/Parser/AbstractXmlParser.php | 6 +++++-
.../core/Classes/Localization/Parser/LocallangXmlParser.php | 6 +++++-
typo3/sysext/core/Classes/Type/File/ImageInfo.php | 6 +++++-
typo3/sysext/core/Classes/Utility/GeneralUtility.php | 6 ++++++
typo3/sysext/core/Tests/FunctionalTestCase.php | 6 +++++-
.../documentation/Classes/Service/DocumentationService.php | 3 +++
.../Classes/Utility/Parser/ExtensionXmlPushParser.php | 3 +++
.../Classes/Utility/Parser/MirrorXmlPushParser.php | 3 +++
typo3/sysext/lang/Classes/Service/TerService.php | 3 +++
.../Tests/Functional/Recycle/AbstractRecycleTestCase.php | 6 +++++-
.../Classes/Controller/SpellCheckingController.php | 3 +++
.../rtehtmlarea/Classes/Extension/MicroDataSchema.php | 3 +++
typo3/sysext/t3editor/Classes/TypoScriptReferenceLoader.php | 3 +++
17 files changed, 69 insertions(+), 5 deletions(-)
diff --git a/typo3/sysext/adodb/Documentation/Index.rst b/typo3/sysext/adodb/Documentation/Index.rst
index d1ef0980c579..a3bae0d49d9a 100644
--- a/typo3/sysext/adodb/Documentation/Index.rst
+++ b/typo3/sysext/adodb/Documentation/Index.rst
@@ -25,6 +25,7 @@ updated to upstream.
- ADOdb: Allow setting NOT NULL/DEFAULT on blob and text columns (67442_) (Upstream pull request: [3]_)
- ADOdb: Table names in sequences broken (64990_)
- ADOdb: PHP7 redefinition of parameter (71244_)
+- Security: XML entity expansion (61269_)
.. [2] https://github.com/ADOdb/ADOdb/commit/85f05a98974ea85ecae943faf230a27afdbaa746
.. [3] https://github.com/ADOdb/ADOdb/pull/118
@@ -38,6 +39,7 @@ updated to upstream.
.. _67442: https://forge.typo3.org/issues/67442
.. _64990: https://forge.typo3.org/issues/64990
.. _71244: https://forge.typo3.org/issues/71244
+.. _61269: https://forge.typo3.org/issues/61269
Diff
diff --git a/typo3/sysext/adodb/adodb/adodb-xmlschema.inc.php b/typo3/sysext/adodb/adodb/adodb-xmlschema.inc.php
index 72a9f9bc757e..fc2c5bc9b9fc 100644
--- a/typo3/sysext/adodb/adodb/adodb-xmlschema.inc.php
+++ b/typo3/sysext/adodb/adodb/adodb-xmlschema.inc.php
@@ -1456,6 +1456,8 @@ class adoSchema {
$this->success = 2;
$xmlParser = $this->create_parser();
+ // Disables the functionality to allow external entities to be loaded when parsing the XML, must be kept
+ $previousValueOfEntityLoader = libxml_disable_entity_loader(true);
// Process the file
while( $data = fread( $fp, 4096 ) ) {
@@ -1468,6 +1470,7 @@ class adoSchema {
}
}
+ libxml_disable_entity_loader($previousValueOfEntityLoader);
xml_parser_free( $xmlParser );
return $this->sqlArray;
@@ -1502,6 +1505,8 @@ class adoSchema {
$this->success = 2;
$xmlParser = $this->create_parser();
+ // Disables the functionality to allow external entities to be loaded when parsing the XML, must be kept
+ $previousValueOfEntityLoader = libxml_disable_entity_loader(true);
if( !xml_parse( $xmlParser, $xmlstring, TRUE ) ) {
die( sprintf(
@@ -1511,6 +1516,7 @@ class adoSchema {
) );
}
+ libxml_disable_entity_loader($previousValueOfEntityLoader);
xml_parser_free( $xmlParser );
return $this->sqlArray;
diff --git a/typo3/sysext/adodb/adodb/adodb-xmlschema03.inc.php b/typo3/sysext/adodb/adodb/adodb-xmlschema03.inc.php
index 3ed5aecbe07f..ba190e7aeafd 100644
--- a/typo3/sysext/adodb/adodb/adodb-xmlschema03.inc.php
+++ b/typo3/sysext/adodb/adodb/adodb-xmlschema03.inc.php
@@ -1612,6 +1612,8 @@ class adoSchema {
$this->success = 2;
$xmlParser = $this->create_parser();
+ // Disables the functionality to allow external entities to be loaded when parsing the XML, must be kept
+ $previousValueOfEntityLoader = libxml_disable_entity_loader(true);
// Process the file
while( $data = fread( $fp, 4096 ) ) {
@@ -1624,6 +1626,7 @@ class adoSchema {
}
}
+ libxml_disable_entity_loader($previousValueOfEntityLoader);
xml_parser_free( $xmlParser );
return $this->sqlArray;
@@ -1659,6 +1662,8 @@ class adoSchema {
$this->success = 2;
$xmlParser = $this->create_parser();
+ // Disables the functionality to allow external entities to be loaded when parsing the XML, must be kept
+ $previousValueOfEntityLoader = libxml_disable_entity_loader(true);
if( !xml_parse( $xmlParser, $xmlstring, TRUE ) ) {
die( sprintf(
@@ -1668,6 +1673,7 @@ class adoSchema {
) );
}
+ libxml_disable_entity_loader($previousValueOfEntityLoader);
xml_parser_free( $xmlParser );
return $this->sqlArray;
diff --git a/typo3/sysext/core/Classes/Imaging/IconProvider/SvgIconProvider.php b/typo3/sysext/core/Classes/Imaging/IconProvider/SvgIconProvider.php
index d4cf6f2dcb89..8e1e5e543870 100644
--- a/typo3/sysext/core/Classes/Imaging/IconProvider/SvgIconProvider.php
+++ b/typo3/sysext/core/Classes/Imaging/IconProvider/SvgIconProvider.php
@@ -93,7 +93,10 @@ class SvgIconProvider implements IconProviderInterface
$svgContent = file_get_contents($source);
$svgContent = preg_replace('/<script[\s\S]*?>[\s\S]*?<\/script>/i', '', $svgContent);
+ // Disables the functionality to allow external entities to be loaded when parsing the XML, must be kept
+ $previousValueOfEntityLoader = libxml_disable_entity_loader(true);
$svgElement = simplexml_load_string($svgContent);
+ libxml_disable_entity_loader($previousValueOfEntityLoader);
// remove xml version tag
$domXml = dom_import_simplexml($svgElement);
diff --git a/typo3/sysext/core/Classes/Localization/Parser/AbstractXmlParser.php b/typo3/sysext/core/Classes/Localization/Parser/AbstractXmlParser.php
index 801c44e58555..bbd5bbdad715 100644
--- a/typo3/sysext/core/Classes/Localization/Parser/AbstractXmlParser.php
+++ b/typo3/sysext/core/Classes/Localization/Parser/AbstractXmlParser.php
@@ -91,7 +91,11 @@ abstract class AbstractXmlParser implements LocalizationParserInterface
*/
protected function parseXmlFile()
{
- $rootXmlNode = simplexml_load_file($this->sourcePath, 'SimpleXMLElement', LIBXML_NOWARNING);
+ $xmlContent = file_get_contents($this->sourcePath);
+ // Disables the functionality to allow external entities to be loaded when parsing the XML, must be kept
+ $previousValueOfEntityLoader = libxml_disable_entity_loader(true);
+ $rootXmlNode = simplexml_load_string($xmlContent, 'SimpleXMLElement', LIBXML_NOWARNING);
+ libxml_disable_entity_loader($previousValueOfEntityLoader);
if (!isset($rootXmlNode) || $rootXmlNode === false) {
throw new InvalidXmlFileException('The path provided does not point to existing and accessible well-formed XML file.', 1278155988);
}
diff --git a/typo3/sysext/core/Classes/Localization/Parser/LocallangXmlParser.php b/typo3/sysext/core/Classes/Localization/Parser/LocallangXmlParser.php
index 328c09c53d7d..d00f8beefbdb 100644
--- a/typo3/sysext/core/Classes/Localization/Parser/LocallangXmlParser.php
+++ b/typo3/sysext/core/Classes/Localization/Parser/LocallangXmlParser.php
@@ -167,7 +167,11 @@ class LocallangXmlParser extends AbstractXmlParser
{
$rootXmlNode = false;
if (file_exists($targetPath)) {
- $rootXmlNode = simplexml_load_file($targetPath, 'SimpleXMLElement', LIBXML_NOWARNING);
+ $xmlContent = file_get_contents($targetPath);
+ // Disables the functionality to allow external entities to be loaded when parsing the XML, must be kept
+ $previousValueOfEntityLoader = libxml_disable_entity_loader(true);
+ $rootXmlNode = simplexml_load_string($xmlContent, 'SimpleXMLElement', LIBXML_NOWARNING);
+ libxml_disable_entity_loader($previousValueOfEntityLoader);
}
if (!isset($rootXmlNode) || $rootXmlNode === false) {
throw new InvalidXmlFileException('The path provided does not point to existing and accessible well-formed XML file (' . $targetPath . ').', 1278155987);
diff --git a/typo3/sysext/core/Classes/Type/File/ImageInfo.php b/typo3/sysext/core/Classes/Type/File/ImageInfo.php
index ef82186fecf0..faddaba1e34a 100644
--- a/typo3/sysext/core/Classes/Type/File/ImageInfo.php
+++ b/typo3/sysext/core/Classes/Type/File/ImageInfo.php
@@ -86,7 +86,11 @@ class ImageInfo extends FileInfo
{
$imagesSizes = array();
- $xml = simplexml_load_file($this->getPathname());
+ $fileContent = file_get_contents($this->getPathname());
+ // Disables the functionality to allow external entities to be loaded when parsing the XML, must be kept
+ $previousValueOfEntityLoader = libxml_disable_entity_loader(true);
+ $xml = simplexml_load_string($fileContent);
+ libxml_disable_entity_loader($previousValueOfEntityLoader);
$xmlAttributes = $xml->attributes();
// First check if width+height are set
diff --git a/typo3/sysext/core/Classes/Utility/GeneralUtility.php b/typo3/sysext/core/Classes/Utility/GeneralUtility.php
index 767729e79bb3..7c5600bb5523 100755
--- a/typo3/sysext/core/Classes/Utility/GeneralUtility.php
+++ b/typo3/sysext/core/Classes/Utility/GeneralUtility.php
@@ -1647,6 +1647,8 @@ class GeneralUtility
*/
public static function xml2tree($string, $depth = 999, $parserOptions = array())
{
+ // Disables the functionality to allow external entities to be loaded when parsing the XML, must be kept
+ $previousValueOfEntityLoader = libxml_disable_entity_loader(true);
$parser = xml_parser_create();
$vals = array();
$index = array();
@@ -1656,6 +1658,7 @@ class GeneralUtility
xml_parser_set_option($parser, $option, $value);
}
xml_parse_into_struct($parser, $string, $vals, $index);
+ libxml_disable_entity_loader($previousValueOfEntityLoader);
if (xml_get_error_code($parser)) {
return 'Line ' . xml_get_current_line_number($parser) . ': ' . xml_error_string(xml_get_error_code($parser));
}
@@ -1895,6 +1898,8 @@ class GeneralUtility
*/
protected static function xml2arrayProcess($string, $NSprefix = '', $reportDocTag = false)
{
+ // Disables the functionality to allow external entities to be loaded when parsing the XML, must be kept
+ $previousValueOfEntityLoader = libxml_disable_entity_loader(true);
// Create parser:
$parser = xml_parser_create();
$vals = array();
@@ -1909,6 +1914,7 @@ class GeneralUtility
xml_parser_set_option($parser, XML_OPTION_TARGET_ENCODING, $theCharset);
// Parse content:
xml_parse_into_struct($parser, $string, $vals, $index);
+ libxml_disable_entity_loader($previousValueOfEntityLoader);
// If error, return error message:
if (xml_get_error_code($parser)) {
return 'Line ' . xml_get_current_line_number($parser) . ': ' . xml_error_string(xml_get_error_code($parser));
diff --git a/typo3/sysext/core/Tests/FunctionalTestCase.php b/typo3/sysext/core/Tests/FunctionalTestCase.php
index 1c629245765d..b5a26ec15e23 100644
--- a/typo3/sysext/core/Tests/FunctionalTestCase.php
+++ b/typo3/sysext/core/Tests/FunctionalTestCase.php
@@ -277,7 +277,11 @@ abstract class FunctionalTestCase extends BaseTestCase
$database = $this->getDatabaseConnection();
- $xml = simplexml_load_file($path);
+ $fileContent = file_get_contents($path);
+ // Disables the functionality to allow external entities to be loaded when parsing the XML, must be kept
+ $previousValueOfEntityLoader = libxml_disable_entity_loader(true);
+ $xml = simplexml_load_string($fileContent);
+ libxml_disable_entity_loader($previousValueOfEntityLoader);
$foreignKeys = array();
/** @var $table \SimpleXMLElement */
diff --git a/typo3/sysext/documentation/Classes/Service/DocumentationService.php b/typo3/sysext/documentation/Classes/Service/DocumentationService.php
index 680bc11da6d4..ce74975dd665 100644
--- a/typo3/sysext/documentation/Classes/Service/DocumentationService.php
+++ b/typo3/sysext/documentation/Classes/Service/DocumentationService.php
@@ -245,7 +245,10 @@ class DocumentationService
*/
protected function parsePackagesXML($string)
{
+ // Disables the functionality to allow external entities to be loaded when parsing the XML, must be kept
+ $previousValueOfEntityLoader = libxml_disable_entity_loader(true);
$data = json_decode(json_encode((array)simplexml_load_string($string)), true);
+ libxml_disable_entity_loader($previousValueOfEntityLoader);
if (count($data) !== 2) {
throw new \TYPO3\CMS\Documentation\Exception\XmlParser('Error in XML parser while decoding packages XML file.', 1374222437);
}
diff --git a/typo3/sysext/extensionmanager/Classes/Utility/Parser/ExtensionXmlPushParser.php b/typo3/sysext/extensionmanager/Classes/Utility/Parser/ExtensionXmlPushParser.php
index 68daf102db14..3a20d88d6ef3 100644
--- a/typo3/sysext/extensionmanager/Classes/Utility/Parser/ExtensionXmlPushParser.php
+++ b/typo3/sysext/extensionmanager/Classes/Utility/Parser/ExtensionXmlPushParser.php
@@ -69,6 +69,8 @@ class ExtensionXmlPushParser extends AbstractExtensionXmlParser
if (!is_resource($this->objXml)) {
throw new \TYPO3\CMS\Extensionmanager\Exception\ExtensionManagerException('Unable to create XML parser.', 1342640663);
}
+ // Disables the functionality to allow external entities to be loaded when parsing the XML, must be kept
+ $previousValueOfEntityLoader = libxml_disable_entity_loader(true);
// keep original character case of XML document
xml_parser_set_option($this->objXml, XML_OPTION_CASE_FOLDING, false);
xml_parser_set_option($this->objXml, XML_OPTION_SKIP_WHITE, false);
@@ -83,6 +85,7 @@ class ExtensionXmlPushParser extends AbstractExtensionXmlParser
throw new \TYPO3\CMS\Extensionmanager\Exception\ExtensionManagerException(sprintf('XML error %s in line %u of file resource %s.', xml_error_string(xml_get_error_code($this->objXml)), xml_get_current_line_number($this->objXml), $file), 1342640703);
}
}
+ libxml_disable_entity_loader($previousValueOfEntityLoader);
xml_parser_free($this->objXml);
}
diff --git a/typo3/sysext/extensionmanager/Classes/Utility/Parser/MirrorXmlPushParser.php b/typo3/sysext/extensionmanager/Classes/Utility/Parser/MirrorXmlPushParser.php
index f21b95ba5cfb..3e77f43b1da9 100644
--- a/typo3/sysext/extensionmanager/Classes/Utility/Parser/MirrorXmlPushParser.php
+++ b/typo3/sysext/extensionmanager/Classes/Utility/Parser/MirrorXmlPushParser.php
@@ -64,6 +64,8 @@ class MirrorXmlPushParser extends AbstractMirrorXmlParser
if (!is_resource($this->objXml)) {
throw new \TYPO3\CMS\Extensionmanager\Exception\ExtensionManagerException('Unable to create XML parser.', 1342641009);
}
+ // Disables the functionality to allow external entities to be loaded when parsing the XML, must be kept
+ $previousValueOfEntityLoader = libxml_disable_entity_loader(true);
// keep original character case of XML document
xml_parser_set_option($this->objXml, XML_OPTION_CASE_FOLDING, false);
xml_parser_set_option($this->objXml, XML_OPTION_SKIP_WHITE, false);
@@ -78,6 +80,7 @@ class MirrorXmlPushParser extends AbstractMirrorXmlParser
throw new \TYPO3\CMS\Extensionmanager\Exception\ExtensionManagerException(sprintf('XML error %s in line %u of file resource %s.', xml_error_string(xml_get_error_code($this->objXml)), xml_get_current_line_number($this->objXml), $file), 1342641011);
}
}
+ libxml_disable_entity_loader($previousValueOfEntityLoader);
xml_parser_free($this->objXml);
}
diff --git a/typo3/sysext/lang/Classes/Service/TerService.php b/typo3/sysext/lang/Classes/Service/TerService.php
index 75595baf7c89..2fbd29548041 100644
--- a/typo3/sysext/lang/Classes/Service/TerService.php
+++ b/typo3/sysext/lang/Classes/Service/TerService.php
@@ -59,12 +59,15 @@ class TerService extends TerUtility implements SingletonInterface
{
// Create parser:
$parser = xml_parser_create();
+ // Disables the functionality to allow external entities to be loaded when parsing the XML, must be kept
+ $previousValueOfEntityLoader = libxml_disable_entity_loader(true);
$values = array();
$index = array();
xml_parser_set_option($parser, XML_OPTION_CASE_FOLDING, 0);
xml_parser_set_option($parser, XML_OPTION_SKIP_WHITE, 0);
// Parse content
xml_parse_into_struct($parser, $string, $values, $index);
+ libxml_disable_entity_loader($previousValueOfEntityLoader);
// If error, return error message
if (xml_get_error_code($parser)) {
$line = xml_get_current_line_number($parser);
diff --git a/typo3/sysext/recycler/Tests/Functional/Recycle/AbstractRecycleTestCase.php b/typo3/sysext/recycler/Tests/Functional/Recycle/AbstractRecycleTestCase.php
index b197e0ade0f6..11eed511da09 100644
--- a/typo3/sysext/recycler/Tests/Functional/Recycle/AbstractRecycleTestCase.php
+++ b/typo3/sysext/recycler/Tests/Functional/Recycle/AbstractRecycleTestCase.php
@@ -92,7 +92,11 @@ abstract class AbstractRecycleTestCase extends \TYPO3\CMS\Core\Tests\FunctionalT
}
$data = array();
- $xml = simplexml_load_file($path);
+ $fileContent = file_get_contents($path);
+ // Disables the functionality to allow external entities to be loaded when parsing the XML, must be kept
+ $previousValueOfEntityLoader = libxml_disable_entity_loader(true);
+ $xml = simplexml_load_string($fileContent);
+ libxml_disable_entity_loader($previousValueOfEntityLoader);
/** @var $table \SimpleXMLElement */
foreach ($xml->children() as $table) {
diff --git a/typo3/sysext/rtehtmlarea/Classes/Controller/SpellCheckingController.php b/typo3/sysext/rtehtmlarea/Classes/Controller/SpellCheckingController.php
index 1babffbfefde..2fdc2aa5f78d 100644
--- a/typo3/sysext/rtehtmlarea/Classes/Controller/SpellCheckingController.php
+++ b/typo3/sysext/rtehtmlarea/Classes/Controller/SpellCheckingController.php
@@ -316,6 +316,8 @@ class SpellCheckingController
$content = GeneralUtility::_POST('content');
// Parsing the input HTML
$parser = xml_parser_create(strtoupper($this->parserCharset));
+ // Disables the functionality to allow external entities to be loaded when parsing the XML, must be kept
+ $previousValueOfEntityLoader = libxml_disable_entity_loader(true);
xml_parser_set_option($parser, XML_OPTION_CASE_FOLDING, 0);
xml_set_object($parser, $this);
if (!xml_set_element_handler($parser, 'startHandler', 'endHandler')) {
@@ -333,6 +335,7 @@ class SpellCheckingController
if (xml_get_error_code($parser)) {
throw new \UnexpectedValueException('Line ' . xml_get_current_line_number($parser) . ': ' . xml_error_string(xml_get_error_code($parser)), 1294585788);
}
+ libxml_disable_entity_loader($previousValueOfEntityLoader);
xml_parser_free($parser);
if ($this->pspell_is_available && !$this->forceCommandMode) {
pspell_clear_session($this->pspell_link);
diff --git a/typo3/sysext/rtehtmlarea/Classes/Extension/MicroDataSchema.php b/typo3/sysext/rtehtmlarea/Classes/Extension/MicroDataSchema.php
index 547b11401df0..e8f03d362f2a 100644
--- a/typo3/sysext/rtehtmlarea/Classes/Extension/MicroDataSchema.php
+++ b/typo3/sysext/rtehtmlarea/Classes/Extension/MicroDataSchema.php
@@ -107,9 +107,12 @@ class MicroDataSchema extends RteHtmlAreaApi
{
$types = array();
$properties = array();
+ // Disables the functionality to allow external entities to be loaded when parsing the XML, must be kept
+ $previousValueOfEntityLoader = libxml_disable_entity_loader(true);
// Load the document
$document = new \DOMDocument();
$document->loadXML($string);
+ libxml_disable_entity_loader($previousValueOfEntityLoader);
if ($document) {
// Scan resource descriptions
$items = $document->getElementsByTagName('Description');
diff --git a/typo3/sysext/t3editor/Classes/TypoScriptReferenceLoader.php b/typo3/sysext/t3editor/Classes/TypoScriptReferenceLoader.php
index 4b1f03c22a38..7948aa00029e 100644
--- a/typo3/sysext/t3editor/Classes/TypoScriptReferenceLoader.php
+++ b/typo3/sysext/t3editor/Classes/TypoScriptReferenceLoader.php
@@ -75,8 +75,11 @@ class TypoScriptReferenceLoader
*/
protected function loadFile($filepath)
{
+ // Disables the functionality to allow external entities to be loaded when parsing the XML, must be kept
+ $previousValueOfEntityLoader = libxml_disable_entity_loader(true);
$this->xmlDoc = new \DOMDocument('1.0', 'utf-8');
$this->xmlDoc->load($filepath);
+ libxml_disable_entity_loader($previousValueOfEntityLoader);
// @TODO: oliver@typo3.org: I guess this is not required here
$this->xmlDoc->saveXML();
}
--
GitLab