T85850-combined-REL1_23.patch
csteipp (Chris Steipp)
Actions

Authored By

	• csteipp
	Mar 24 2015, 8:07 PM

Size

4 KB

Referenced Files

None

Subscribers

None

T85850-combined-REL1_23.patch
View Options

	From d6fe2a96e4e56cd8f23f69e898feabc8366e1159 Mon Sep 17 00:00:00 2001
	From: csteipp <csteipp@wikimedia.org>
	Date: Mon, 23 Mar 2015 18:03:24 -0700
	Subject: [PATCH] SECURITY: Don't allow embedded application/xml in SVG's

	Fix for iSEC-WMF1214-11 and issue reported by Cure 53, which got
	around our blacklist on embedded href targets. Use a whitelist instead.

	Bug: T85850
	---
	includes/upload/UploadBase.php \| 20 ++++++++++----------
	tests/phpunit/includes/upload/UploadBaseTest.php \| 13 +++++++++++++
	2 files changed, 23 insertions(+), 10 deletions(-)

	diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
	index 3db2653..cf3e67d 100644
	--- a/includes/upload/UploadBase.php
	+++ b/includes/upload/UploadBase.php
	@@ -1315,16 +1315,16 @@ abstract class UploadBase {
	}
	}

	- # href with embedded svg as target
	- if ( $stripped == 'href' && preg_match( '!data:[^,]image/svg[^,],!sim', $value ) ) {
	- wfDebug( __METHOD__ . ": Found href to embedded svg \"<$strippedElement '$attrib'='$value'...\" in uploaded file.\n" );
	- return true;
	- }
	-
	- # href with embedded (text/xml) svg as target
	- if ( $stripped == 'href' && preg_match( '!data:[^,]text/xml[^,],!sim', $value ) ) {
	- wfDebug( __METHOD__ . ": Found href to embedded svg \"<$strippedElement '$attrib'='$value'...\" in uploaded file.\n" );
	- return true;
	+ # only allow data: targets that should be safe. This prevents vectors like,
	+ # image/svg, text/xml, application/xml, and text/html, which can contain scripts
	+ if ( $stripped == 'href' && strncasecmp( 'data:', $value, 5 ) === 0 ) {
	+ // rfc2397 parameters. This is only slightly slower than (;[\w;]+)*.
	+ $parameters = '(?>;[a-zA-Z0-9\!#$&\'+.^_`{\|}~-]+=(?>[a-zA-Z0-9\!#$&\'+.^_`{\|}~-]+\|"(?>[\0-\x0c\x0e-\x21\x23-\x5b\x5d-\x7f]+\|\\\\[\0-\x7f])"))(?:;base64)?';
	+ if ( !preg_match( "!^data:\s*image/(gif\|jpeg\|jpg\|png)$parameters,!i", $value ) ) {
	+ wfDebug( __METHOD__ . ": Found href to unwhitelisted data: uri "
	+ . "\"<$strippedElement '$attrib'='$value'...\" in uploaded file.\n" );
	+ return true;
	+ }
	}

	# Change href with animate from (http://html5sec.org/#137). This doesn't seem
	diff --git a/tests/phpunit/includes/upload/UploadBaseTest.php b/tests/phpunit/includes/upload/UploadBaseTest.php
	index 475513e..ac8cc43 100644
	--- a/tests/phpunit/includes/upload/UploadBaseTest.php
	+++ b/tests/phpunit/includes/upload/UploadBaseTest.php
	@@ -168,6 +168,12 @@ class UploadBaseTest extends MediaWikiTestCase {
	'SVG with javascript xlink (http://html5sec.org/#87)'
	),
	array(
	+ '<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"><use xlink:href="data:application/xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHhtbG5zOnhsaW5rPSJodHRwOi8vd3d3LnczLm9yZy8xOTk5L3hsaW5rIj4KPGRlZnM+CjxjaXJjbGUgaWQ9InRlc3QiIHI9IjUwIiBjeD0iMTAwIiBjeT0iMTAwIiBzdHlsZT0iZmlsbDogI0YwMCI+CjxzZXQgYXR0cmlidXRlTmFtZT0iZmlsbCIgYXR0cmlidXRlVHlwZT0iQ1NTIiBvbmJlZ2luPSdhbGVydChkb2N1bWVudC5jb29raWUpJwpvbmVuZD0nYWxlcnQoIm9uZW5kIiknIHRvPSIjMDBGIiBiZWdpbj0iMXMiIGR1cj0iNXMiIC8+CjwvY2lyY2xlPgo8L2RlZnM+Cjx1c2UgeGxpbms6aHJlZj0iI3Rlc3QiLz4KPC9zdmc+#test"/> </svg>',
	+ true,
	+ true,
	+ 'SVG with Opera image xlink (http://html5sec.org/#88 - c)'
	+ ),
	+ array(
	'<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"> <animation xlink:href="javascript:alert(1)"/> </svg>',
	true,
	true,
	@@ -342,6 +348,13 @@ class UploadBaseTest extends MediaWikiTestCase {
	true,
	'SVG with remote background image using image() (bug 69008)'
	),
	+ array(
	+ // As reported by Cure53
	+ '<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"> <a xlink:href="data:text/html;charset=utf-8;base64, PHNjcmlwdD5hbGVydChkb2N1bWVudC5kb21haW4pPC9zY3JpcHQ%2BDQo%3D"> <circle r="400" fill="red"></circle> </a> </svg>',
	+ true,
	+ true,
	+ 'SVG with data:text/html link target (firefox only)'
	+ ),

	// Test good, but strange files that we want to allow
	array(
	--
	1.8.4.5

File Metadata

Mime Type: text/x-diff
Storage Engine: blob
Storage Format: Raw Data
Storage Handle: 100637
Default Alt Text: T85850-combined-REL1_23.patch (4 KB)

T85850-combined-REL1_23.patchcsteipp (Chris Steipp)Actions

T85850-combined-REL1_23.patchView Options

File Metadata

Event Timeline

T85850-combined-REL1_23.patch
csteipp (Chris Steipp)
Actions

T85850-combined-REL1_23.patch
View Options