From: Brian Wolff <bawolff+wn@gmail.com>
Date: Mon, 28 Jul 2014 21:20:30 +0000 (-0300)
Subject: Relax filter attribute filtering to allow self-referential urls
X-Git-Tag: 1.34.0-rc.0~3123
X-Git-Url: http://git.cyclocoop.org/data/File:Image2.gif?a=commitdiff_plain;h=8c7b635162123ef8328da309b62795d07c17fff6;p=lhc%2Fweb%2Fwiklou.git

Relax filter attribute filtering to allow self-referential urls

The filter attribute will often have things like filter="url( #foo )"
These local to the file filters in svgs should be fine (We already
disallow non-local xlink:href attributes on <filter> elements). In
fact, users can already do the exact same thing by doing:
style="filter: url( #foo )"

Bug: 67044
Change-Id: Ib25328c160c0d5ea7e01dc84616b76e1b9dcd0eb
---

diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index c7dbf835b9..a579b69ccc 100644
--- a/includes/upload/UploadBase.php
+++ b/includes/upload/UploadBase.php
@@ -1740,9 +1740,10 @@ abstract class UploadBase {
 			}
 
 			# image filters can pull in url, which could be svg that executes scripts
+			# Only allow url( "#foo" ). Do not allow url( http://example.com )
 			if ( $strippedElement == 'image'
 				&& $stripped == 'filter'
-				&& preg_match( '!url\s*\(!sim', $value )
+				&& preg_match( '!url\s*\(\s*["\']?[^#]!sim', $value )
 			) {
 				wfDebug( __METHOD__ . ": Found image filter with url: "
 					. "\"<$strippedElement $stripped='$value'...\" in uploaded file.\n" );
diff --git a/tests/phpunit/includes/upload/UploadBaseTest.php b/tests/phpunit/includes/upload/UploadBaseTest.php
index a80262e932..58c69e3229 100644
--- a/tests/phpunit/includes/upload/UploadBaseTest.php
+++ b/tests/phpunit/includes/upload/UploadBaseTest.php
@@ -143,8 +143,8 @@ class UploadBaseTest extends MediaWikiTestCase {
 			// html5sec SVG vectors
 			[
 				'<svg xmlns="http://www.w3.org/2000/svg"><script>alert(1)</script></svg>',
-				true,
-				true,
+				true, /* SVG is well formed */
+				true, /* Evil SVG detected */
 				'Script tag in svg (http://html5sec.org/#47)'
 			],
 			[
@@ -509,7 +509,20 @@ class UploadBaseTest extends MediaWikiTestCase {
 				true,
 				false,
 				'DTD with aliased entities apos (Should be allowed)'
-			]
+			],
+			[
+				'<svg xmlns="http://www.w3.org/2000/svg"><g filter="url( \'#foo\' )"></g></svg>',
+				true,
+				false,
+				'SVG with local filter (T69044)'
+			],
+			[
+				'<svg xmlns="http://www.w3.org/2000/svg"><g filter="url( http://example.com/#foo )"></g></svg>',
+				true,
+				true,
+				'SVG with non-local filter (T69044)'
+			],
+
 		];
 		// phpcs:enable
 	}