From c9cb0baa528ecbc563c226c16907e5db30a30599 Mon Sep 17 00:00:00 2001 From: csteipp Date: Mon, 3 Feb 2014 16:49:48 -0800 Subject: [PATCH] SECURITY: Disallow non-whitelisted namespaces Disallow uploading non-whitelisted namespaces. Also disallow iframe elements. User will get an error including the namespace name if they use a non- whitelisted namespace. Bug: 60771 Change-Id: Id5c022543184b19b77ad32d9a8a0c2dbbc5e9038 --- includes/upload/UploadBase.php | 72 ++++++++++++++++++++++++++++++++++++-- languages/messages/MessagesEn.php | 1 + languages/messages/MessagesQqq.php | 8 +++++ 3 files changed, 79 insertions(+), 2 deletions(-) diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php index 67bffc3..db7a24e 100644 --- a/includes/upload/UploadBase.php +++ b/includes/upload/UploadBase.php @@ -42,7 +42,7 @@ abstract class UploadBase { protected $mFilteredName, $mFinalExtension; protected $mLocalFile, $mFileSize, $mFileProps; protected $mBlackListedExtensions; - protected $mJavaDetected; + protected $mJavaDetected, $mSVGNSError; protected static $safeXmlEncodings = array( 'UTF-8', 'ISO-8859-1', 'ISO-8859-2', 'UTF-16', 'UTF-32' ); @@ -1168,6 +1168,7 @@ abstract class UploadBase { * @return mixed false of the file is verified (does not contain scripts), array otherwise. */ protected function detectScriptInSvg( $filename ) { + $this->mSVGNSError = false; $check = new XmlTypeCheck( $filename, array( $this, 'checkSvgScriptCallback' ), @@ -1178,6 +1179,9 @@ abstract class UploadBase { // Invalid xml (bug 58553) return array( 'uploadinvalidxml' ); } elseif ( $check->filterMatch ) { + if ( $this->mSVGNSError ) { + return array( 'uploadscriptednamespace', $this->mSVGNSError ); + } return array( 'uploadscripted' ); } return false; @@ -1204,7 +1208,51 @@ abstract class UploadBase { * @return bool */ public function checkSvgScriptCallback( $element, $attribs ) { - $strippedElement = $this->stripXmlNamespace( $element ); + list( $namespace, $strippedElement ) = $this->splitXmlNamespace( $element ); + + static $validNamespaces = array( + '', + 'adobe:ns:meta/', + 'http://creativecommons.org/ns#', + 'http://inkscape.sourceforge.net/dtd/sodipodi-0.dtd', + 'http://ns.adobe.com/adobeillustrator/10.0/', + 'http://ns.adobe.com/adobesvgviewerextensions/3.0/', + 'http://ns.adobe.com/extensibility/1.0/', + 'http://ns.adobe.com/flows/1.0/', + 'http://ns.adobe.com/illustrator/1.0/', + 'http://ns.adobe.com/imagereplacement/1.0/', + 'http://ns.adobe.com/pdf/1.3/', + 'http://ns.adobe.com/photoshop/1.0/', + 'http://ns.adobe.com/saveforweb/1.0/', + 'http://ns.adobe.com/variables/1.0/', + 'http://ns.adobe.com/xap/1.0/', + 'http://ns.adobe.com/xap/1.0/g/', + 'http://ns.adobe.com/xap/1.0/g/img/', + 'http://ns.adobe.com/xap/1.0/mm/', + 'http://ns.adobe.com/xap/1.0/rights/', + 'http://ns.adobe.com/xap/1.0/stype/dimensions#', + 'http://ns.adobe.com/xap/1.0/stype/font#', + 'http://ns.adobe.com/xap/1.0/stype/manifestitem#', + 'http://ns.adobe.com/xap/1.0/stype/resourceevent#', + 'http://ns.adobe.com/xap/1.0/stype/resourceref#', + 'http://ns.adobe.com/xap/1.0/t/pg/', + 'http://purl.org/dc/elements/1.1/', + 'http://purl.org/dc/elements/1.1', + 'http://schemas.microsoft.com/visio/2003/svgextensions/', + 'http://sodipodi.sourceforge.net/dtd/sodipodi-0.dtd', + 'http://web.resource.org/cc/', + 'http://www.freesoftware.fsf.org/bkchem/cdml', + 'http://www.inkscape.org/namespaces/inkscape', + 'http://www.w3.org/1999/02/22-rdf-syntax-ns#', + 'http://www.w3.org/2000/svg', + ); + + if ( !in_array( $namespace, $validNamespaces ) ) { + wfDebug( __METHOD__ . ": Non-svg namespace '$namespace' in uploaded file.\n" ); + // @TODO return a status object to a closure in XmlTypeCheck, for MW1.21+ + $this->mSVGNSError = $namespace; + return true; + } /* * check for elements that can contain javascript @@ -1226,6 +1274,13 @@ abstract class UploadBase { return true; } + # Block iframes, in case they pass the namespace check + if ( $strippedElement == 'iframe' ) { + wfDebug( __METHOD__ . ": iframe in uploaded file.\n" ); + return true; + } + + foreach ( $attribs as $attrib => $value ) { $stripped = $this->stripXmlNamespace( $attrib ); $value = strtolower( $value ); @@ -1300,6 +1355,19 @@ abstract class UploadBase { } /** + * Divide the element name passed by the xml parser to the callback into URI and prifix. + * @param $name string + * @return array containing the namespace URI and prefix + */ + private static function splitXmlNamespace( $element ) { + // 'http://www.w3.org/2000/svg:script' -> array( 'http://www.w3.org/2000/svg', 'script' ) + $parts = explode( ':', strtolower( $element ) ); + $name = array_pop( $parts ); + $ns = implode( ':', $parts ); + return array( $ns, $name ); + } + + /** * @param $name string * @return string */ diff --git a/languages/messages/MessagesEn.php b/languages/messages/MessagesEn.php index 5d42c83..545132f 100644 --- a/languages/messages/MessagesEn.php +++ b/languages/messages/MessagesEn.php @@ -2348,6 +2348,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.', +'uploadscriptednamespace' => 'This SVG file contains an illegal namespace \'$1\'', 'uploadinvalidxml' => 'The XML in the uploaded file could not be parsed.', 'uploadvirus' => 'The file contains a virus! Details: $1', diff --git a/languages/messages/MessagesQqq.php b/languages/messages/MessagesQqq.php index 048756b..97443df 100644 --- a/languages/messages/MessagesQqq.php +++ b/languages/messages/MessagesQqq.php @@ -4123,6 +4123,14 @@ See also: * {{msg-mw|zip-wrong-format}} * {{msg-mw|uploadjava}} * {{msg-mw|uploadvirus}}', +'uploadscriptednamespace' => 'Used as error message when uploading a file. This error is specific to SVG files, when they include a namespace that has not been whitelisted. + +Parameters: +* $1 - the invalid namespace name +See also: +* {{msg-mw|zip-wrong-format}} +* {{msg-mw|uploadjava}} +* {{msg-mw|uploadvirus}}', 'uploadvirus' => 'Error message displayed when uploaded file contains a virus. Parameters: -- 1.8.4.5