From: csteipp <csteipp@wikimedia.org>
Date: Thu, 19 Dec 2013 20:52:18 +0000 (-0800)
Subject: SECURITY: Return error on invalid XML for SVGs
X-Git-Tag: 1.31.0-rc.0~17260
X-Git-Url: https://git.cyclocoop.org/%7B%7B%20url_for%28?a=commitdiff_plain;h=b4260e2daa2c815d74d4ff1a6d65663e06bd5369;p=lhc%2Fweb%2Fwiklou.git

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.

This patch is rebased on top of bug 57550.

Bug: 58553
Change-Id: I32661a27d7417cc2c69b844c805b190d6486d17a
---

diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index 8ee36a67f9..67bffc3238 100644
--- a/includes/upload/UploadBase.php
+++ b/includes/upload/UploadBase.php
@@ -474,9 +474,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;
 				}
 			}
 		}
@@ -1164,7 +1165,7 @@ 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(
@@ -1173,7 +1174,13 @@ abstract class UploadBase {
 			true,
 			array( 'processing_instruction_handler' => 'UploadBase::checkSvgPICallback' )
 		);
-		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 0728d281ca..41a38e4a9a 100644
--- a/languages/messages/MessagesEn.php
+++ b/languages/messages/MessagesEn.php
@@ -2343,6 +2343,7 @@ You should ask someone with the ability to view suppressed file data to review t
 '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 15a3ff3188..1f47bf03e8 100644
--- a/languages/messages/MessagesQqq.php
+++ b/languages/messages/MessagesQqq.php
@@ -4073,6 +4073,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: