* Fixed XSS vulnerability introduced by r49833. Only pre-release versions of MediaWik...
authorTim Starling <tstarling@users.mediawiki.org>
Mon, 17 Aug 2009 13:23:45 +0000 (13:23 +0000)
committerTim Starling <tstarling@users.mediawiki.org>
Mon, 17 Aug 2009 13:23:45 +0000 (13:23 +0000)
* Refactored the IE script entry point security check into WebRequest::isPathInfoBad(). Use the standard CGI variable PATH_INFO to do this check instead of the various potential non-standard solutions. Made the check fairly permissive to avoid a repeat of bug 13049 due to broken CGI setups especially with cgi.fix_pathinfo=0. This should theoretically be very portable and secure, but I have not tested it widely.
* Removed Chris Wrinn from the credits since his patch was wrong and has been removed.
* Made the error message more informative.

CREDITS
RELEASE-NOTES
api.php
includes/RawPage.php
includes/WebRequest.php
mwScriptLoader.php

diff --git a/CREDITS b/CREDITS
index bbdf82d..3016f48 100644 (file)
--- a/CREDITS
+++ b/CREDITS
@@ -63,7 +63,6 @@ following names for their contribution to the product.
 * Brent G
 * Brianna Laugher
 * Carlin
-* Chris Wrinn
 * church of emacs
 * Dan Nessett
 * Daniel Arnold
index 6582dc1..58ad369 100644 (file)
@@ -406,6 +406,8 @@ this. Was used when mwEmbed was going to be an extension.
 * (bug 20273) Fix broken output when no pages are found in the content
   namespaces
 * (bug 20265) Make AncientPages and UnusedFiles work on SQLite
+* Fixed XSS vulnerability for Internet Explorer clients (only pre-release 
+  versions of MediaWiki were affected).
 
 == API changes in 1.16 ==
 
diff --git a/api.php b/api.php
index ec805fc..16657ca 100644 (file)
--- a/api.php
+++ b/api.php
@@ -49,16 +49,10 @@ wfProfileIn('api.php');
 // which will end up triggering HTML detection and execution, hence
 // XSS injection and all that entails.
 //
-// Ensure that all access is through the canonical entry point...
-//
-if( isset( $_SERVER['SCRIPT_NAME'] ) ) {
-       $url = $_SERVER['SCRIPT_NAME'];
-} else {
-       $url = $_SERVER['URL'];
-}
-if( strcmp( "$wgScriptPath/api$wgScriptExtension", $url ) ) {
+if( $wgRequest->isPathInfoBad() ) {
        wfHttpError( 403, 'Forbidden',
-               'API must be accessed through the primary script entry point.' );
+               'Invalid file extension found in PATH_INFO. ' . 
+               'The API must be accessed through the primary script entry point.' );
        return;
 }
 
index ea698bc..3e38144 100644 (file)
@@ -109,19 +109,9 @@ class RawPage {
        }
 
        function view() {
-               global $wgOut, $wgScript;
-
-               $url = wfGetScriptUrl();
-               if( $url == '' ) {
-                       # This will make the next check fail with a confusing error
-                       # message, so we should mention it separately.
-                       wfHttpError( 500, 'Internal Server Error',
-                               "\$_SERVER['URL'] is not set.  Perhaps you're using CGI" .
-                               " and haven't set cgi.fix_pathinfo = 1 in php.ini?" );
-                       return;
-               }
+               global $wgOut, $wgScript, $wgRequest;
 
-               if( strcmp( $wgScript, $url ) ) {
+               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
@@ -135,6 +125,7 @@ class RawPage {
                        #
                        # Just return a 403 Forbidden and get it over with.
                        wfHttpError( 403, 'Forbidden',
+                               'Invalid file extension found in PATH_INFO. ' . 
                                'Raw pages must be accessed through the primary script entry point.' );
                        return;
                }
index 784dc0f..157f093 100644 (file)
@@ -670,6 +670,33 @@ class WebRequest {
        public function setSessionData( $key, $data ) {
                $_SESSION[$key] = $data;
        }
+
+       /**
+        * 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
+        * is not HTML but may be detected as such.
+        *
+        * Various past attempts to use the URL to make this check have generally 
+        * run up against the fact that CGI does not provide a standard method to 
+        * determine the URL. PATH_INFO may be mangled (e.g. if cgi.fix_pathinfo=0), 
+        * but only by prefixing it with the script name and maybe some other stuff, 
+        * the extension is not mangled. So this should be a reasonably portable 
+        * way to perform this security check.
+        */
+       public function isPathInfoBad() {
+               global $wgScriptExtension;
+               
+               if ( !isset( $_SERVER['PATH_INFO'] ) ) {
+                       return false;
+               }
+               $pi = $_SERVER['PATH_INFO'];
+               $dotPos = strrpos( $pi, '.' );
+               if ( $dotPos === false ) {
+                       return false;
+               }
+               $ext = substr( $pi, $dotPos );
+               return !in_array( $ext, array( $wgScriptExtension, '.php', '.php5' ) );
+       }
 }
 
 /**
@@ -740,4 +767,8 @@ class FauxRequest extends WebRequest {
                $this->notImplemented( __METHOD__ );
        }
 
+       public function isPathInfoBad() {
+               return false;
+       }
+
 }
index 8637cc8..c884127 100644 (file)
@@ -30,8 +30,9 @@ require_once('includes/WebStart.php');
 wfProfileIn( 'mwScriptLoader.php' );
 
 
-if( strpos( wfGetScriptUrl(), "mwScriptLoader{$wgScriptExtension}" ) === false ){
+if( $wgRequest->isPathInfoBad() ){
        wfHttpError( 403, 'Forbidden',
+               'Invalid file extension found in PATH_INFO. ' . 
                'mwScriptLoader must be accessed through the primary script entry point.' );
        return;
 }
@@ -51,4 +52,4 @@ if ( !$wgEnableScriptLoader ) {
 $myScriptLoader = new jsScriptLoader();
 $myScriptLoader->doScriptLoader();
 
-wfProfileOut( 'mwScriptLoader.php' );
\ No newline at end of file
+wfProfileOut( 'mwScriptLoader.php' );