Commit 0103f48c authored by Benni Mack's avatar Benni Mack Committed by Oliver Hader
Browse files

[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's avatarOliver Hader <oliver.hader@typo3.org>
Tested-by: Oliver Hader's avatarOliver Hader <oliver.hader@typo3.org>
parent 3163daa0
......@@ -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
......
......@@ -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;
......
......@@ -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;
......
......@@ -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);
......
......@@ -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);
}
......
......@@ -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);
......
......@@ -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
......
......@@ -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));
......
......@@ -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 */
......
......@@ -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);
}
......
......@@ -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);
}
......
......@@ -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);
}
......
......@@ -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);
......
......@@ -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) {
......
......@@ -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);
......
......@@ -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');
......
......@@ -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();
}
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment