From b5f491fb6d5d2a0d808223c898ba99b98637fa71 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 8c3f1740a2..6da8250b7e 100644 --- a/includes/upload/UploadBase.php +++ b/includes/upload/UploadBase.php @@ -1424,11 +1424,10 @@ abstract class UploadBase { } } - # 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 8c5c9236a9..c027af6bd4 100644 --- a/tests/phpunit/includes/upload/UploadBaseTest.php +++ b/tests/phpunit/includes/upload/UploadBaseTest.php @@ -279,6 +279,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( -- 2.20.1