From 35c97c6e1cf395d34fea6f61d1e20cf94614bfac Mon Sep 17 00:00:00 2001
From: csteipp <csteipp@wikimedia.org>
Date: Mon, 16 Dec 2013 13:56:34 -0800
Subject: [PATCH] SECURITY: Return error on invalid XML for SVGs

Return an error from UploadBase::detectScriptInSvg when the svg has
XML that cannot be parsed. Usually the XML is invalid, or the parser has
run out of memory trying to parse the file.

Bug: 58553
Change-Id: I19131613aa519d883b0901c1d347eb8557487761
---
 includes/upload/UploadBase.php     | 15 +++++++++++----
 languages/messages/MessagesEn.php  |  1 +
 languages/messages/MessagesQqq.php |  1 +
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index a6c3421..fc547b8 100644
--- a/includes/upload/UploadBase.php
+++ b/includes/upload/UploadBase.php
@@ -476,9 +476,10 @@ abstract class UploadBase {
 				return array( 'uploadscripted' );
 			}
 			if ( $this->mFinalExtension == 'svg' || $mime == 'image/svg+xml' ) {
-				if ( $this->detectScriptInSvg( $this->mTempPath ) ) {
+				$svgStatus = $this->detectScriptInSvg( $this->mTempPath );
+				if ( $svgStatus !== false ) {
 					wfProfileOut( __METHOD__ );
-					return array( 'uploadscripted' );
+					return $svgStatus;
 				}
 			}
 		}
@@ -1155,11 +1156,17 @@ abstract class UploadBase {
 
 	/**
 	 * @param $filename string
-	 * @return bool
+	 * @return mixed false of the file is verified (does not contain scripts), array otherwise.
 	 */
 	protected function detectScriptInSvg( $filename ) {
 		$check = new XmlTypeCheck( $filename, array( $this, 'checkSvgScriptCallback' ) );
-		return $check->filterMatch;
+		if ( $check->wellFormed !== true ) {
+			// Invalid xml (bug 58553)
+			return array( 'uploadinvalidxml' );
+		} elseif ( $check->filterMatch ) {
+			return array( 'uploadscripted' );
+		}
+		return false;
 	}
 
 	/**
diff --git a/languages/messages/MessagesEn.php b/languages/messages/MessagesEn.php
index 0fe59e0..ba1424d 100644
--- a/languages/messages/MessagesEn.php
+++ b/languages/messages/MessagesEn.php
@@ -2334,6 +2334,7 @@ You should check that file's deletion history before proceeding to re-upload it.
 'php-uploaddisabledtext'      => 'File uploads are disabled in PHP.
 Please check the file_uploads setting.',
 'uploadscripted'              => 'This file contains HTML or script code that may be erroneously interpreted by a web browser.',
+'uploadinvalidxml'            => 'The XML in the uploaded file could not be parsed.',
 'uploadvirus'                 => 'The file contains a virus!
 Details: $1',
 'uploadjava'                  => 'The file is a ZIP file that contains a Java .class file.
diff --git a/languages/messages/MessagesQqq.php b/languages/messages/MessagesQqq.php
index e3e574a..c1265a4 100644
--- a/languages/messages/MessagesQqq.php
+++ b/languages/messages/MessagesQqq.php
@@ -4049,6 +4049,7 @@ See also:
 * {{msg-mw|zip-wrong-format}}
 * {{msg-mw|uploadjava}}
 * {{msg-mw|uploadvirus}}',
+'uploadinvalidxml' => 'Error message displayed when the uploaded file contains XML that cannot be properly parsed and checked.',
 'uploadvirus' => 'Error message displayed when uploaded file contains a virus.
 
 Parameters:
-- 
1.8.4