(bug 28840) If the query string hits bug 28235, redirect to a safer URL instead of...
authorTim Starling <tstarling@users.mediawiki.org>
Fri, 3 Jun 2011 05:32:51 +0000 (05:32 +0000)
committerTim Starling <tstarling@users.mediawiki.org>
Fri, 3 Jun 2011 05:32:51 +0000 (05:32 +0000)
api.php
includes/RawPage.php
includes/WebRequest.php
load.php

diff --git a/api.php b/api.php
index 978e8d4..7fff048 100644 (file)
--- a/api.php
+++ b/api.php
@@ -61,18 +61,7 @@ wfProfileIn( 'api.php' );
 $starttime = microtime( true );
 
 // URL safety checks
-//
-// See RawPage.php for details; summary is that MSIE can override the
-// Content-Type if it sees a recognized extension on the URL, such as
-// might be appended via PATH_INFO after 'api.php'.
-//
-// Some data formats can end up containing unfiltered user-provided data
-// which will end up triggering HTML detection and execution, hence
-// XSS injection and all that entails.
-//
-if ( $wgRequest->isPathInfoBad() ) {
-       wfHttpError( 403, 'Forbidden',
-               'Invalid file extension found in PATH_INFO or QUERY_STRING.' );
+if ( !$wgRequest->checkUrlExtension() ) {
        return;
 }
 
index 5804bf9..c6c4a66 100644 (file)
@@ -118,22 +118,8 @@ class RawPage {
        function view() {
                global $wgOut, $wgRequest;
 
-               if( $wgRequest->isPathInfoBad() ) {
-                       # Internet Explorer will ignore the Content-Type header if it
-                       # thinks it sees a file extension it recognizes. Make sure that
-                       # all raw requests are done through the script node, which will
-                       # have eg '.php' and should remain safe.
-                       #
-                       # We used to redirect to a canonical-form URL as a general
-                       # backwards-compatibility / good-citizen nice thing. However
-                       # a lot of servers are set up in buggy ways, resulting in
-                       # redirect loops which hang the browser until the CSS load
-                       # times out.
-                       #
-                       # Just return a 403 Forbidden and get it over with.
-                       wfHttpError( 403, 'Forbidden',
-                               'Invalid file extension found in PATH_INFO or QUERY_STRING. ' .
-                               'Raw pages must be accessed through the primary script entry point.' );
+               if( !$wgRequest->checkUrlExtension() ) {
+                       $wgOut->disable();
                        return;
                }
 
index 68cb73e..9ede547 100644 (file)
@@ -766,6 +766,69 @@ class WebRequest {
                $_SESSION[$key] = $data;
        }
 
+       /**
+        * Check if Internet Explorer will detect an incorrect cache extension in 
+        * PATH_INFO or QUERY_STRING. If the request can't be allowed, show an error
+        * message or redirect to a safer URL. Returns true if the URL is OK, and
+        * false if an error message has been shown and the request should be aborted.
+        */
+       public function checkUrlExtension() {
+               $query = isset( $_SERVER['QUERY_STRING'] ) ? $_SERVER['QUERY_STRING'] : '';
+               if ( self::isUrlExtensionBad( $query ) ) {
+                       if ( !$this->wasPosted() ) {
+                               if ( $this->attemptExtensionSecurityRedirect() ) {
+                                       return false;
+                               }
+                       }
+                       wfHttpError( 403, 'Forbidden',
+                               'Invalid file extension found in the query string.' );
+                       
+                       return false;
+               }
+
+               if ( $this->isPathInfoBad() ) {
+                       wfHttpError( 403, 'Forbidden',
+                               'Invalid file extension found in PATH_INFO.' );
+                       return false;
+               }
+               return true;
+       }
+
+       /**
+        * Attempt to redirect to a URL with a QUERY_STRING that's not dangerous in 
+        * IE 6. Returns true if it was successful, false otherwise.
+        */
+       protected function attemptExtensionSecurityRedirect() {
+               $url = self::fixUrlForIE6( $this->getFullRequestURL() );
+               if ( $url === false ) {
+                       return false;
+               }
+
+               header( 'Location: ' . $url );
+               header( 'Content-Type: text/html' );
+               $encUrl = htmlspecialchars( $url );
+               echo <<<HTML
+<html>
+<head>
+<title>Security redirect</title>
+</head>
+<body>
+<h1>Security redirect</h1>
+<p>
+We can't serve non-HTML content from the URL you have requested, because 
+Internet Explorer would interpret it as an incorrect and potentially dangerous
+content type.</p>
+<p>Instead, please use <a href="$encUrl">this URL</a>, which is the same as the URL you have requested, except that 
+"&amp;*" is appended. This prevents Internet Explorer from seeing a bogus file 
+extension.
+</p>
+</body>
+</html>
+HTML;
+               echo "\n";
+               return true;
+       }
+
        /**
         * Returns true if the PATH_INFO ends with an extension other than a script
         * extension. This could confuse IE for scripts that send arbitrary data which
@@ -884,8 +947,18 @@ class WebRequest {
                if ( !isset( $_SERVER['QUERY_STRING'] ) ) {
                        return false;
                }
+               return self::isUrlExtensionBad( $_SERVER['QUERY_STRING'] );
+       }
+
+       /**
+        * The same as WebRequest::isQueryStringBad() except as a static function.
+        */
+       public static function isUrlExtensionBad( $query ) {
+               if ( strval( $query ) === '' ) {
+                       return false;
+               }
 
-               $extension = self::findIE6Extension( $_SERVER['QUERY_STRING'] );
+               $extension = self::findIE6Extension( $query );
                if ( strval( $extension ) === '' ) {
                        /* No extension or empty extension (false/'') */
                        return false;
@@ -901,6 +974,31 @@ class WebRequest {
                return (bool)preg_match( '/^[a-zA-Z0-9_-]+$/', $extension );
        }
 
+       /**
+        * Returns a variant of $url which will pass isUrlExtensionBad() but has the 
+        * same GET parameters, or false if it can't figure one out.
+        */
+       public static function fixUrlForIE6( $url ) {
+               $questionPos = strpos( $url, '?' );
+               if ( $questionPos === false || $questionPos === strlen( $url ) - 1 ) {
+                       return $url;
+               }
+
+               $beforeQuery = substr( $url, 0, $questionPos + 1 );
+               $query = substr( $url, $questionPos + 1 );
+               // Multiple question marks cause problems. Encode the second and 
+               // subsequent question mark.
+               $query = str_replace( '?', '%3E', $query );
+               // Append an invalid path character so that IE6 won't see the end of the
+               // query string as an extension
+               $query .= '&*';
+               if ( self::isUrlExtensionBad( $query ) ) {
+                       // Avoid a redirect loop
+                       return false;
+               }
+               return $beforeQuery . $query;
+       }
+
        /**
         * Parse the Accept-Language header sent by the client into an array
         * @return array( languageCode => q-value ) sorted by q-value in descending order
index 2fc48fc..74af68f 100644 (file)
--- a/load.php
+++ b/load.php
@@ -45,17 +45,7 @@ if ( isset( $_SERVER['MW_COMPILED'] ) ) {
 wfProfileIn( 'load.php' );
 
 // URL safety checks
-//
-// See RawPage.php for details; summary is that MSIE can override the
-// Content-Type if it sees a recognized extension on the URL, such as
-// might be appended via PATH_INFO after 'load.php'.
-//
-// Some resources can contain HTML-like strings (e.g. in messages)
-// which will end up triggering HTML detection and execution.
-//
-if ( $wgRequest->isPathInfoBad() ) {
-       wfHttpError( 403, 'Forbidden',
-               'Invalid file extension found in PATH_INFO or QUERY_STRING.' );
+if ( !$wgRequest->checkUrlExtension() ) {
        return;
 }