Merge "OutputPage: Tiny tweak to jQuery.ready inline script"

diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index a79526e..6da8250 100644 (file)
--- a/includes/upload/UploadBase.php
+++ b/includes/upload/UploadBase.php
@@ -1412,27 +1412,22 @@ 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
-                       # 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" );
@@ -1524,7 +1519,7 @@ abstract class UploadBase {
        private static function checkCssFragment( $value ) {
 
                # Forbid external stylesheets, for both reliability and to protect viewer's privacy
-               if ( strpos( $value, '@import' ) !== false ) {
+               if ( stripos( $value, '@import' ) !== false ) {
                        return true;
                }
 

diff --git a/tests/phpunit/includes/upload/UploadBaseTest.php b/tests/phpunit/includes/upload/UploadBaseTest.php
index 63ad8c0..c027af6 100644 (file)
--- a/tests/phpunit/includes/upload/UploadBaseTest.php
+++ b/tests/phpunit/includes/upload/UploadBaseTest.php
@@ -162,6 +162,12 @@ class UploadBaseTest extends MediaWikiTestCase {
                                true,
                                '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,
@@ -273,6 +279,18 @@ class UploadBaseTest extends MediaWikiTestCase {
                                true,
                                'SVG with animate from (http://html5sec.org/#137)'
                        ),
+                       array(
+                               '<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"> <a><text y="1em">Click me</text> <animate attributeName="xlink:href" values="javascript:alert(\'Bang!\')" begin="0s" dur="0.1s" fill="freeze" /> </a></svg>',
+                               true,
+                               true,
+                               'SVG with animate xlink:href (http://html5sec.org/#137)'
+                       ),
+                       array(
+                               '<svg xmlns="http://www.w3.org/2000/svg" xmlns:y="http://www.w3.org/1999/xlink"> <a y:href="#"> <text y="1em">Click me</text> <animate attributeName="y:href" values="javascript:alert(\'Bang!\')" begin="0s" dur="0.1s" fill="freeze" /> </a> </svg>',
+                               true,
+                               true,
+                               'SVG with animate y:href (http://html5sec.org/#137)'
+                       ),
 
                        // Other hostile SVG's
                        array(
@@ -305,6 +323,12 @@ class UploadBaseTest extends MediaWikiTestCase {
                                true,
                                'SVG with @import in style element and child element (bug 69008#c11)'
                        ),
+                       array(
+                               '<svg xmlns="http://www.w3.org/2000/svg" viewBox="6 3 177 153" xmlns:xlink="http://www.w3.org/1999/xlink"> <style>@imporT "https://fonts.googleapis.com/css?family=Bitter:700&amp;text=WebPlatform.org";</style> <g transform="translate(-.5,-.5)"> <text fill="#474747" x="95" y="150" text-anchor="middle" font-family="Bitter" font-size="20" font-weight="bold">WebPlatform.org</text> </g> </svg>',
+                               true,
+                               true,
+                               'SVG with case-insensitive @import in style element (bug T85349)'
+                       ),
                        array(
                                '<svg xmlns="http://www.w3.org/2000/svg"> <rect width="100" height="100" style="background-image:url(https://www.google.com/images/srpr/logo11w.png)"/> </svg>',
                                true,
@@ -331,6 +355,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(