CSteipp has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/59346


Change subject: Disable external entities in XMLReader
......................................................................

Disable external entities in XMLReader

Temporarily disable loading entities in XMLReader when calling read()
with libxml_disable_entity_loader(true).

(cherry-pick from 1ed76385c31f44c589f6e5a99c9c56f1f3f76728)

bug: 46859
Change-Id: I0b2ef270f15c7b4da17edee680bf7e2410919915
---
M includes/media/SVGMetadataExtractor.php
1 file changed, 12 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/46/59346/1

diff --git a/includes/media/SVGMetadataExtractor.php 
b/includes/media/SVGMetadataExtractor.php
index db9f05f..57ff097 100644
--- a/includes/media/SVGMetadataExtractor.php
+++ b/includes/media/SVGMetadataExtractor.php
@@ -68,10 +68,15 @@
                        $this->reader->open( $source, null, LIBXML_NOERROR | 
LIBXML_NOWARNING );
                }
 
-               // 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.
+               // 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. 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;
@@ -85,9 +90,11 @@
                        $this->read();
                } catch( Exception $e ) {
                        wfRestoreWarnings();
+                       libxml_disable_entity_loader( $oldDisable );
                        throw $e;
                }
                wfRestoreWarnings();
+               libxml_disable_entity_loader( $oldDisable );
        }
 
        /**
@@ -100,7 +107,7 @@
        /**
         * Read the SVG
         */
-       public function read() {
+       protected function read() {
                $keepReading = $this->reader->read();
 
                /* Skip until first element */

-- 
To view, visit https://gerrit.wikimedia.org/r/59346
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I0b2ef270f15c7b4da17edee680bf7e2410919915
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: REL1_19
Gerrit-Owner: CSteipp <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to