Alex Monk has submitted this change and it was merged.
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).
bug: 46859
Change-Id: I0b2ef270f15c7b4da17edee680bf7e2410919915
---
M includes/media/SVGMetadataExtractor.php
1 file changed, 12 insertions(+), 5 deletions(-)
Approvals:
Alex Monk: Verified; Looks good to me, approved
diff --git a/includes/media/SVGMetadataExtractor.php
b/includes/media/SVGMetadataExtractor.php
index 851fe42..c6f63fd 100644
--- a/includes/media/SVGMetadataExtractor.php
+++ b/includes/media/SVGMetadataExtractor.php
@@ -74,10 +74,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;
@@ -99,9 +104,11 @@
// 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 );
}
/**
@@ -115,7 +122,7 @@
* Read the SVG
* @return bool
*/
- public function read() {
+ protected function read() {
$keepReading = $this->reader->read();
/* Skip until first element */
--
To view, visit https://gerrit.wikimedia.org/r/59340
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I0b2ef270f15c7b4da17edee680bf7e2410919915
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: REL1_20
Gerrit-Owner: CSteipp <[email protected]>
Gerrit-Reviewer: Alex Monk <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
