From 858634de026432a4fed548f0c014452bd40cbd23 Mon Sep 17 00:00:00 2001 From: csteipp Date: Thu, 7 Jan 2016 08:13:16 -0800 Subject: [PATCH] SECURITY: Don't use m modifier when checking link prefix SVG filter incorrectly used the m modifier when checking if an href attribute started with 'https?://', incorrectly matching attributes such as, "javascript:alert(' http://foo')". Bug: T122653 Change-Id: I41291fff344241cad3171f3e8050de99b62a2296 --- includes/upload/UploadBase.php | 3 +-- tests/phpunit/includes/upload/UploadBaseTest.php | 6 ++++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php index f8624d0..95d1d06 100644 --- a/includes/upload/UploadBase.php +++ b/includes/upload/UploadBase.php @@ -1291,7 +1291,6 @@ abstract class UploadBase { * @return bool */ public function checkSvgScriptCallback( $element, $attribs, $data = null ) { - list( $namespace, $strippedElement ) = $this->splitXmlNamespace( $element ); // We specifically don't include: @@ -1400,7 +1399,7 @@ abstract class UploadBase { && strpos( $value, '#' ) !== 0 ) { if ( !( $strippedElement === 'a' - && preg_match( '!^https?://!im', $value ) ) + && preg_match( '!^https?://!i', $value ) ) ) { wfDebug( __METHOD__ . ": Found href attribute <$strippedElement " . "'$attrib'='$value' in uploaded file.\n" ); diff --git a/tests/phpunit/includes/upload/UploadBaseTest.php b/tests/phpunit/includes/upload/UploadBaseTest.php index 90051ee..cf86702 100644 --- a/tests/phpunit/includes/upload/UploadBaseTest.php +++ b/tests/phpunit/includes/upload/UploadBaseTest.php @@ -374,6 +374,12 @@ class UploadBaseTest extends MediaWikiTestCase { false, 'SVG with external entity' ), + array( + " ", + true, + false, + 'SVG with javascript link with newline (T122653)' + ), // Test good, but strange files that we want to allow array( -- 1.8.4.5