From de80f8f0aa89ac85f8875515982636e5bb08766b Mon Sep 17 00:00:00 2001
From: Brian Wolff <bawolff+wn@gmail.com>
Date: Mon, 28 Nov 2016 23:34:24 +0000
Subject: [PATCH] Security: Load default attribute values for SVG validation

This prevents someone bypassing filter by using default
attribute values in internal dtd subset.

Note, browsers ignore external dtds, so we do too.

Issue reported by Cassiogomes11.

Bug: T151735
Change-Id: I7cb4690f759ad97e70e06e560978b6207d84c446
---
 includes/libs/mime/XmlTypeCheck.php              | 19 +++++++++++++++++--
 tests/phpunit/includes/upload/UploadBaseTest.php | 18 ++++++++++++++++++
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/includes/libs/mime/XmlTypeCheck.php b/includes/libs/mime/XmlTypeCheck.php
index 3958f8c..b33220e 100644
--- a/includes/libs/mime/XmlTypeCheck.php
+++ b/includes/libs/mime/XmlTypeCheck.php
@@ -138,9 +138,9 @@ class XmlTypeCheck {
 	private function validateFromInput( $xml, $isFile ) {
 		$reader = new XMLReader();
 		if ( $isFile ) {
-			$s = $reader->open( $xml, null, LIBXML_NOERROR | LIBXML_NOWARNING );
+			$s = $reader->open( $xml, null, LIBXML_NOERROR | LIBXML_NOWARNING | LIBXML_DTDATTR );
 		} else {
-			$s = $reader->XML( $xml, null, LIBXML_NOERROR | LIBXML_NOWARNING );
+			$s = $reader->XML( $xml, null, LIBXML_NOERROR | LIBXML_NOWARNING | LIBXML_DTDATTR );
 		}
 		if ( $s !== true ) {
 			// Couldn't open the XML
@@ -171,6 +171,21 @@ class XmlTypeCheck {
 	}
 
 	public function XmlErrorHandler( $errno, $errstr ) {
+		// Horrible hack. HHVM seems to complain about the svg dtd
+		// even though it isn't planning to fetch it anyways.
+		$svgdtd = [
+			'http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd',
+			'http://www.w3.org/TR/2001/REC-SVG-20010904/DTD/svg10.dtd',
+			'http://www.w3.org/Graphics/SVG/1.1/DTD/svg11-basic.dtd',
+			'http://www.w3.org/Graphics/SVG/1.1/DTD/svg11-tiny.dtd'
+		];
+		if ( $errno === 2 ) {
+			foreach( $svgdtd as $dtd ) {
+				if ( $errstr === "Protocol 'http' for external XML entity '$dtd' is disabled for security reasons. This may be changed using the hhvm.libxml.ext_entity_whitelist ini setting." ) {
+					return;
+				}
+			}
+		}
 		$this->wellFormed = false;
 	}
 
diff --git a/tests/phpunit/includes/upload/UploadBaseTest.php b/tests/phpunit/includes/upload/UploadBaseTest.php
index 6be272f..dd2be8a 100644
--- a/tests/phpunit/includes/upload/UploadBaseTest.php
+++ b/tests/phpunit/includes/upload/UploadBaseTest.php
@@ -375,6 +375,12 @@ class UploadBaseTest extends MediaWikiTestCase {
 				'SVG with external entity'
 			],
 			[
+				'<?xml version="1.0"?> <!DOCTYPE svg [ <!ENTITY foo SYSTEM "data:text/plain,bar"> ]> <svg xmlns="http://www.w3.org/2000/svg" version="1.1"> <desc>&foo;</desc> <rect width="300" height="100" style="fill:rgb(0,0,255);stroke-width:1;stroke:rgb(0,0,2)" /> </svg>',
+				false,
+				false,
+				'SVG with data: uri external entity'
+			],
+			[
 				"<svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:xlink=\"http://www.w3.org/1999/xlink\"> <g> <a xlink:href=\"javascript:alert('1&#10;https://google.com')\"> <rect width=\"300\" height=\"100\" style=\"fill:rgb(0,0,255);stroke-width:1;stroke:rgb(0,0,2)\" /> </a> </g> </svg>",
 				true,
 				true,
@@ -393,6 +399,18 @@ class UploadBaseTest extends MediaWikiTestCase {
 				false,
 				'SVG with local urls, including filter: in style'
 			],
+			[
+				'<?xml version="1.0" encoding="UTF-8" standalone="no"?><!DOCTYPE x [<!ATTLIST image x:href CDATA "data:image/png,foo" onerror CDATA "alert(\'XSSED = \'+document.domain)" onload CDATA "alert(\'XSSED = \'+document.domain)"> ]> <svg xmlns:h="http://www.w3.org/1999/xhtml" xmlns:x="http://www.w3.org/1999/xlink" xmlns="http://www.w3.org/2000/svg"> <image /> </svg>',
+				true,
+				true,
+				'SVG with evil default attribute values'
+			],
+			[
+				'<?xml version="1.0" encoding="UTF-8" standalone="no"?><!DOCTYPE svg SYSTEM "data:application/xml-dtd;base64,PCFET0NUWVBFIHN2ZyBbPCFBVFRMSVNUIGltYWdlIHg6aHJlZiBDREFUQSAiZGF0YTppbWFnZS9wbmcsZm9vIiBvbmVycm9yIENEQVRBICJhbGVydCgnWFNTRUQgPSAnK2RvY3VtZW50LmRvbWFpbikiIG9ubG9hZCBDREFUQSAiYWxlcnQoJ1hTU0VEID0gJytkb2N1bWVudC5kb21haW4pIj4gXT4K"><svg xmlns:h="http://www.w3.org/1999/xhtml" xmlns:x="http://www.w3.org/1999/xlink" xmlns="http://www.w3.org/2000/svg"> <image /> </svg>',
+				true,
+				false,
+				'SVG with an evil external dtd, but should not be loaded'
+			]
 		];
 		// @codingStandardsIgnoreEnd
 	}
-- 
1.9.5 (Apple Git-50.3)