From 1ed76385c31f44c589f6e5a99c9c56f1f3f76728 Mon Sep 17 00:00:00 2001 From: csteipp Date: Mon, 15 Apr 2013 13:44:23 -0700 Subject: [PATCH] Disable external entities in XMLReader Temporarily disable loading entities in XMLReader when calling read() with libxml_disable_entity_loader(true). bug: 46859 Change-Id: I0b2ef270f15c7b4da17edee680bf7e2410919915 --- includes/media/SVGMetadataExtractor.php | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/includes/media/SVGMetadataExtractor.php b/includes/media/SVGMetadataExtractor.php index 128bba36b1..592403ad5d 100644 --- a/includes/media/SVGMetadataExtractor.php +++ b/includes/media/SVGMetadataExtractor.php @@ -78,7 +78,12 @@ class SVGReader { // Expand entities, since Adobe Illustrator uses them for xmlns // attributes (bug 31719). Note that libxml2 has some protection // against large recursive entity expansions so this is not as - // insecure as it might appear to be. + // insecure as it might appear to be. However, it is still extremely + // insecure. It's necessary to wrap any read() calls with + // libxml_disable_entity_loader() to avoid arbitrary local file + // inclusion, or even arbitrary code execution if the expect + // extension is installed (bug 46859). + $oldDisable = libxml_disable_entity_loader( true ); $this->reader->setParserProperty( XMLReader::SUBST_ENTITIES, true ); $this->metadata['width'] = self::DEFAULT_WIDTH; @@ -100,9 +105,11 @@ class SVGReader { // Note, if this happens, the width/height will be taken to be 0x0. // Should we consider it the default 512x512 instead? wfRestoreWarnings(); + libxml_disable_entity_loader( $oldDisable ); throw $e; } wfRestoreWarnings(); + libxml_disable_entity_loader( $oldDisable ); } /** @@ -117,7 +124,7 @@ class SVGReader { * @throws MWException * @return bool */ - public function read() { + protected function read() { $keepReading = $this->reader->read(); /* Skip until first element */ -- 2.20.1