From ac1e9cd952eaeed733ac59154f595185c83d403e Mon Sep 17 00:00:00 2001 From: csteipp Date: Tue, 13 Jan 2015 16:48:01 -0800 Subject: [PATCH] SECURITY: Fix animate blacklist The blacklist should prevent animating any element's xlink:href to a javascript url. Bug: T86711 Change-Id: Ia9e9192165fdfe1701f22605eee0b0e5c9137d5a --- includes/upload/UploadBase.php | 7 +++---- tests/phpunit/includes/upload/UploadBaseTest.php | 12 ++++++++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php index a8a38c7..c26070b 100644 --- a/includes/upload/UploadBase.php +++ b/includes/upload/UploadBase.php @@ -1426,11 +1426,10 @@ abstract class UploadBase { return true; } - # Change href with animate from (http://html5sec.org/#137). This doesn't seem - # possible without embedding the svg, but filter here in case. - if ( $stripped == 'from' + # Change href with animate from (http://html5sec.org/#137). + if ( $stripped === 'attributename' && $strippedElement === 'animate' - && !preg_match( '!^https?://!im', $value ) + && $this->stripXmlNamespace( $value ) == 'href' ) { wfDebug( __METHOD__ . ": Found animate that might be changing href using from " . "\"<$strippedElement '$attrib'='$value'...\" in uploaded file.\n" ); diff --git a/tests/phpunit/includes/upload/UploadBaseTest.php b/tests/phpunit/includes/upload/UploadBaseTest.php index f23b264..da3399e 100644 --- a/tests/phpunit/includes/upload/UploadBaseTest.php +++ b/tests/phpunit/includes/upload/UploadBaseTest.php @@ -274,6 +274,18 @@ class UploadBaseTest extends MediaWikiTestCase { true, 'SVG with animate from (http://html5sec.org/#137)' ), + array( + ' Click me ', + true, + true, + 'SVG with animate xlink:href (http://html5sec.org/#137)' + ), + array( + ' Click me ', + true, + true, + 'SVG with animate y:href (http://html5sec.org/#137)' + ), // Other hostile SVG's array( -- 1.8.4.5