* Moved most of the bug 28235 code out to a separate library class, since I was running out of distinct function names.
* Merged the QUERY_STRING and PATH_INFO security checks, since they are dealing with the exact same problem. Removed WebRequest::isQueryStringBad().
* Deal with img_auth.php by having it specify what extension it expects to be streaming out. This extension can then be compared with the extension that IE might detect.
wfForbidden('img-auth-accessdenied','img-auth-public');
}
+$matches = WebRequest::getPathInfo();
+$path = $matches['title'];
+
// Check for bug 28235: QUERY_STRING overriding the correct extension
-if ( $wgRequest->isQueryStringBad() )
+$dotPos = strpos( $path, '.' );
+$whitelist = array();
+if ( $dotPos !== false ) {
+ $whitelist[] = substr( $path, $dotPos + 1 );
+}
+if ( !$wgRequest->checkUrlExtension( $whitelist ) )
{
- wfForbidden( 'img-auth-accessdenied', 'img-auth-bad-query-string' );
-}
+ return;
+}
-$matches = WebRequest::getPathInfo();
-$path = $matches['title'];
$filename = realpath( $wgUploadDirectory . $path );
$realUpload = realpath( $wgUploadDirectory );
'CSSJanus' => 'includes/libs/CSSJanus.php',
'CSSMin' => 'includes/libs/CSSMin.php',
'IEContentAnalyzer' => 'includes/libs/IEContentAnalyzer.php',
+ 'IEUrlExtension' => 'includes/libs/IEUrlExtension.php',
'JavaScriptMinifier' => 'includes/libs/JavaScriptMinifier.php',
# includes/media
* 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 ) ) {
+ public function checkUrlExtension( $extWhitelist = array() ) {
+ global $wgScriptExtension;
+ $extWhitelist[] = ltrim( $wgScriptExtension, '.' );
+ if ( IEUrlExtension::areServerVarsBad( $_SERVER, $extWhitelist ) ) {
if ( !$this->wasPosted() ) {
- if ( $this->attemptExtensionSecurityRedirect() ) {
+ $newUrl = IEUrlExtension::fixUrlForIE6(
+ $this->getFullRequestURL(), $extWhitelist );
+ if ( $newUrl !== false ) {
+ $this->doSecurityRedirect( $newUrl );
return false;
}
}
wfHttpError( 403, 'Forbidden',
- 'Invalid file extension found in the query string.' );
+ 'Invalid file extension found in the path info or 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;
- }
-
+ protected function doSecurityRedirect( $url ) {
header( 'Location: ' . $url );
header( 'Content-Type: text/html' );
$encUrl = htmlspecialchars( $url );
* Also checks for anything that looks like a file extension at the end of
* QUERY_STRING, since IE 6 and earlier will use this to get the file type
* if there was no dot before the question mark (bug 28235).
- */
- public function isPathInfoBad() {
- global $wgScriptExtension;
-
- if ( $this->isQueryStringBad() ) {
- return true;
- }
-
- 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' ) );
- }
-
- /**
- * Determine what extension IE6 will infer from a certain query string.
- * If the URL has an extension before the question mark, IE6 will use
- * that and ignore the query string, but per the comment at
- * isPathInfoBad() we don't have a reliable way to determine the URL,
- * so isPathInfoBad() just passes in the query string for $url.
- * All entry points have safe extensions (php, php5) anyway, so
- * checking the query string is possibly overly paranoid but never
- * insecure.
*
- * The criteria for finding an extension are as follows:
- * - a possible extension is a dot followed by one or more characters not
- * in <>\"/:|?.#
- * - if we find a possible extension followed by the end of the string or
- * a #, that's our extension
- * - if we find a possible extension followed by a ?, that's our extension
- * - UNLESS it's exe, dll or cgi, in which case we ignore it and continue
- * searching for another possible extension
- * - if we find a possible extension followed by a dot or another illegal
- * character, we ignore it and continue searching
- *
- * @param $url string URL
- * @return mixed Detected extension (string), or false if none found
- */
- public static function findIE6Extension( $url ) {
- $pos = 0;
- $hashPos = strpos( $url, '#' );
- if ( $hashPos !== false ) {
- $urlLength = $hashPos;
- } else {
- $urlLength = strlen( $url );
- }
- $remainingLength = $urlLength;
- while ( $remainingLength > 0 ) {
- // Skip ahead to the next dot
- $pos += strcspn( $url, '.', $pos, $remainingLength );
- if ( $pos >= $urlLength ) {
- // End of string, we're done
- return false;
- }
-
- // We found a dot. Skip past it
- $pos++;
- $remainingLength = $urlLength - $pos;
-
- // Check for illegal characters in our prospective extension,
- // or for another dot
- $nextPos = $pos + strcspn( $url, "<>\\\"/:|?*.", $pos, $remainingLength );
- if ( $nextPos >= $urlLength ) {
- // No illegal character or next dot
- // We have our extension
- return substr( $url, $pos, $urlLength - $pos );
- }
- if ( $url[$nextPos] === '?' ) {
- // We've found a legal extension followed by a question mark
- // If the extension is NOT exe, dll or cgi, return it
- $extension = substr( $url, $pos, $nextPos - $pos );
- if ( strcasecmp( $extension, 'exe' ) && strcasecmp( $extension, 'dll' ) &&
- strcasecmp( $extension, 'cgi' ) )
- {
- return $extension;
- }
- // Else continue looking
- }
- // We found an illegal character or another dot
- // Skip to that character and continue the loop
- $pos = $nextPos + 1;
- $remainingLength = $urlLength - $pos;
- }
- return false;
- }
-
- /**
- * Check for a bad query string, which IE 6 will use as a potentially
- * insecure cache file extension. See bug 28235. Returns true if the
- * request should be disallowed.
- *
- * @return Boolean
+ * @deprecated Use checkUrlExtension().
*/
- public function isQueryStringBad() {
- 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( $query );
- if ( strval( $extension ) === '' ) {
- /* No extension or empty extension (false/'') */
- return false;
- }
-
- /* Only consider the extension understood by IE to be potentially
- * dangerous if it is made of normal characters (so it is more
- * likely to be registered with an application)
- * Compromise with api.php convenience. Considers for instance
- * that no sane application will register a dangerous file type
- * in an extension containing an ampersand.
- */
- 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;
+ public function isPathInfoBad( $extWhitelist = array() ) {
+ global $wgScriptExtension;
+ $extWhitelist[] = ltrim( $wgScriptExtension, '.' );
+ return IEUrlExtension::areServerVarsBad( $_SERVER, $extWhitelist );
}
/**
return $this->session;
}
- public function isPathInfoBad() {
+ public function isPathInfoBad( $extWhitelist = array() ) {
return false;
}
+
+ public function checkUrlExtension( $extWhitelist = array() ) {
+ return true;
+ }
}
--- /dev/null
+<?php
+
+/**
+ * Internet Explorer derives a cache filename from a URL, and then in certain
+ * circumstances, uses the extension of the resulting file to determine the
+ * content type of the data, ignoring the Content-Type header.
+ *
+ * This can be a problem, especially when non-HTML content is sent by MediaWiki,
+ * and Internet Explorer interprets it as HTML, exposing an XSS vulnerability.
+ *
+ * Usually the script filename (e.g. api.php) is present in the URL, and this
+ * makes Internet Explorer think the extension is a harmless script extension.
+ * But Internet Explorer 6 and earlier allows the script extension to be
+ * obscured by encoding the dot as "%2E".
+ *
+ * This class contains functions which help in detecting and dealing with this
+ * situation.
+ *
+ * Checking the URL for a bad extension is somewhat complicated due to the fact
+ * that CGI doesn't provide a standard method to determine the URL. Instead it
+ * is necessary to pass a subset of $_SERVER variables, which we then attempt
+ * to use to guess parts of the URL.
+ */
+class IEUrlExtension {
+ /**
+ * Check a subset of $_SERVER (or the whole of $_SERVER if you like)
+ * to see if it indicates that the request was sent with a bad file
+ * extension. Returns true if the request should be denied or modified,
+ * false otherwise. The relevant $_SERVER elements are:
+ *
+ * - SERVER_SOFTWARE
+ * - REQUEST_URI
+ * - QUERY_STRING
+ * - PATH_INFO
+ *
+ * If the a variable is unset in $_SERVER, it should be unset in $vars.
+ *
+ * @param $vars A subset of $_SERVER.
+ * @param $extWhitelist Extensions which are allowed, assumed harmless.
+ */
+ public static function areServerVarsBad( $vars, $extWhitelist = array() ) {
+ // Check QUERY_STRING or REQUEST_URI
+ if ( isset( $vars['SERVER_SOFTWARE'] )
+ && isset( $vars['REQUEST_URI'] )
+ && self::haveUndecodedRequestUri( $vars['SERVER_SOFTWARE'] ) )
+ {
+ $urlPart = $vars['REQUEST_URI'];
+ } elseif ( isset( $vars['QUERY_STRING'] ) ) {
+ $urlPart = $vars['QUERY_STRING'];
+ } else {
+ $urlPart = '';
+ }
+
+ if ( self::isUrlExtensionBad( $urlPart, $extWhitelist ) ) {
+ return true;
+ }
+
+ // Some servers have PATH_INFO but not REQUEST_URI, so we check both
+ // to be on the safe side.
+ if ( isset( $vars['PATH_INFO'] )
+ && self::isUrlExtensionBad( $vars['PATH_INFO'], $extWhitelist ) )
+ {
+ return true;
+ }
+
+ // All checks passed
+ return false;
+ }
+
+ /**
+ * Given a right-hand portion of a URL, determine whether IE would detect
+ * a potentially harmful file extension.
+ *
+ * @param $urlPart The right-hand portion of a URL
+ * @param $extWhitelist An array of file extensions which may occur in this
+ * URL, and which should be allowed.
+ * @return bool
+ */
+ public static function isUrlExtensionBad( $urlPart, $extWhitelist = array() ) {
+ if ( strval( $urlPart ) === '' ) {
+ return false;
+ }
+
+ $extension = self::findIE6Extension( $urlPart );
+ if ( strval( $extension ) === '' ) {
+ // No extension or empty extension
+ return false;
+ }
+
+ if ( in_array( $extension, array( 'php', 'php5' ) ) ) {
+ // Script extension, OK
+ return false;
+ }
+ if ( in_array( $extension, $extWhitelist ) ) {
+ // Whitelisted extension
+ return false;
+ }
+
+ if ( !preg_match( '/^[a-zA-Z0-9_-]+$/', $extension ) ) {
+ // Non-alphanumeric extension, unlikely to be registered.
+ //
+ // The regex above is known to match all registered file extensions
+ // in a default Windows XP installation. It's important to allow
+ // extensions with ampersands and percent signs, since that reduces
+ // the number of false positives substantially.
+ return false;
+ }
+
+ // Possibly bad extension
+ return true;
+ }
+
+ /**
+ * 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, $extWhitelist = array() ) {
+ $questionPos = strpos( $url, '?' );
+ if ( $questionPos === false ) {
+ $beforeQuery = $url . '?';
+ $query = '';
+ } elseif ( $questionPos === strlen( $url ) - 1 ) {
+ $beforeQuery = $url;
+ $query = '';
+ } else {
+ $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 .= '&*';
+ // Put the URL back together
+ $url = $beforeQuery . $query;
+ if ( self::isUrlExtensionBad( $url, $extWhitelist ) ) {
+ // Avoid a redirect loop
+ return false;
+ }
+ return $url;
+ }
+
+ /**
+ * Determine what extension IE6 will infer from a certain query string.
+ * If the URL has an extension before the question mark, IE6 will use
+ * that and ignore the query string, but per the comment at
+ * isPathInfoBad() we don't have a reliable way to determine the URL,
+ * so isPathInfoBad() just passes in the query string for $url.
+ * All entry points have safe extensions (php, php5) anyway, so
+ * checking the query string is possibly overly paranoid but never
+ * insecure.
+ *
+ * The criteria for finding an extension are as follows:
+ * - a possible extension is a dot followed by one or more characters not
+ * in <>\"/:|?.#
+ * - if we find a possible extension followed by the end of the string or
+ * a #, that's our extension
+ * - if we find a possible extension followed by a ?, that's our extension
+ * - UNLESS it's exe, dll or cgi, in which case we ignore it and continue
+ * searching for another possible extension
+ * - if we find a possible extension followed by a dot or another illegal
+ * character, we ignore it and continue searching
+ *
+ * @param $url string URL
+ * @return mixed Detected extension (string), or false if none found
+ */
+ public static function findIE6Extension( $url ) {
+ $pos = 0;
+ $hashPos = strpos( $url, '#' );
+ if ( $hashPos !== false ) {
+ $urlLength = $hashPos;
+ } else {
+ $urlLength = strlen( $url );
+ }
+ $remainingLength = $urlLength;
+ while ( $remainingLength > 0 ) {
+ // Skip ahead to the next dot
+ $pos += strcspn( $url, '.', $pos, $remainingLength );
+ if ( $pos >= $urlLength ) {
+ // End of string, we're done
+ return false;
+ }
+
+ // We found a dot. Skip past it
+ $pos++;
+ $remainingLength = $urlLength - $pos;
+
+ // Check for illegal characters in our prospective extension,
+ // or for another dot
+ $nextPos = $pos + strcspn( $url, "<>\\\"/:|?*.", $pos, $remainingLength );
+ if ( $nextPos >= $urlLength ) {
+ // No illegal character or next dot
+ // We have our extension
+ return substr( $url, $pos, $urlLength - $pos );
+ }
+ if ( $url[$nextPos] === '?' ) {
+ // We've found a legal extension followed by a question mark
+ // If the extension is NOT exe, dll or cgi, return it
+ $extension = substr( $url, $pos, $nextPos - $pos );
+ if ( strcasecmp( $extension, 'exe' ) && strcasecmp( $extension, 'dll' ) &&
+ strcasecmp( $extension, 'cgi' ) )
+ {
+ return $extension;
+ }
+ // Else continue looking
+ }
+ // We found an illegal character or another dot
+ // Skip to that character and continue the loop
+ $pos = $nextPos + 1;
+ $remainingLength = $urlLength - $pos;
+ }
+ return false;
+ }
+
+ /**
+ * When passed the value of $_SERVER['SERVER_SOFTWARE'], this function
+ * returns true if that server is known to have a REQUEST_URI variable
+ * with %2E not decoded to ".". On such a server, it is possible to detect
+ * whether the script filename has been obscured.
+ *
+ * The function returns false if the server is not known to have this
+ * behaviour. Microsoft IIS in particular is known to decode escaped script
+ * filenames.
+ *
+ * SERVER_SOFTWARE typically contains either a plain string such as "Zeus",
+ * or a specification in the style of a User-Agent header, such as
+ * "Apache/1.3.34 (Unix) mod_ssl/2.8.25 OpenSSL/0.9.8a PHP/4.4.2"
+ *
+ * @param $serverSoftware
+ * @return bool
+ *
+ */
+ public static function haveUndecodedRequestUri( $serverSoftware ) {
+ static $whitelist = array(
+ 'Apache',
+ 'Zeus',
+ 'LiteSpeed' );
+ if ( preg_match( '/^(.*?)($|\/| )/', $serverSoftware, $m ) ) {
+ return in_array( $m[1], $whitelist );
+ } else {
+ return false;
+ }
+ }
+
+}
<?php
/**
- * Tests for WebRequest::findIE6Extension
+ * Tests for IEUrlExtension::findIE6Extension
*/
-class FindIE6ExtensionTest extends MediaWikiTestCase {
+class IEUrlExtensionTest extends MediaWikiTestCase {
function testSimple() {
$this->assertEquals(
'y',
- WebRequest::findIE6Extension( 'x.y' ),
+ IEUrlExtension::findIE6Extension( 'x.y' ),
'Simple extension'
);
}
function testSimpleNoExt() {
$this->assertEquals(
'',
- WebRequest::findIE6Extension( 'x' ),
+ IEUrlExtension::findIE6Extension( 'x' ),
'No extension'
);
}
function testEmpty() {
$this->assertEquals(
'',
- WebRequest::findIE6Extension( '' ),
+ IEUrlExtension::findIE6Extension( '' ),
'Empty string'
);
}
function testQuestionMark() {
$this->assertEquals(
'',
- WebRequest::findIE6Extension( '?' ),
+ IEUrlExtension::findIE6Extension( '?' ),
'Question mark only'
);
}
function testExtQuestionMark() {
$this->assertEquals(
'x',
- WebRequest::findIE6Extension( '.x?' ),
+ IEUrlExtension::findIE6Extension( '.x?' ),
'Extension then question mark'
);
}
function testQuestionMarkExt() {
$this->assertEquals(
'x',
- WebRequest::findIE6Extension( '?.x' ),
+ IEUrlExtension::findIE6Extension( '?.x' ),
'Question mark then extension'
);
}
function testInvalidChar() {
$this->assertEquals(
'',
- WebRequest::findIE6Extension( '.x*' ),
+ IEUrlExtension::findIE6Extension( '.x*' ),
'Extension with invalid character'
);
}
function testInvalidCharThenExtension() {
$this->assertEquals(
'x',
- WebRequest::findIE6Extension( '*.x' ),
+ IEUrlExtension::findIE6Extension( '*.x' ),
'Invalid character followed by an extension'
);
}
function testMultipleQuestionMarks() {
$this->assertEquals(
'c',
- WebRequest::findIE6Extension( 'a?b?.c?.d?e?f' ),
+ IEUrlExtension::findIE6Extension( 'a?b?.c?.d?e?f' ),
'Multiple question marks'
);
}
function testExeException() {
$this->assertEquals(
'd',
- WebRequest::findIE6Extension( 'a?b?.exe?.d?.e' ),
+ IEUrlExtension::findIE6Extension( 'a?b?.exe?.d?.e' ),
'.exe exception'
);
}
function testExeException2() {
$this->assertEquals(
'exe',
- WebRequest::findIE6Extension( 'a?b?.exe' ),
+ IEUrlExtension::findIE6Extension( 'a?b?.exe' ),
'.exe exception 2'
);
}
function testHash() {
$this->assertEquals(
'',
- WebRequest::findIE6Extension( 'a#b.c' ),
+ IEUrlExtension::findIE6Extension( 'a#b.c' ),
'Hash character preceding extension'
);
}
function testHash2() {
$this->assertEquals(
'',
- WebRequest::findIE6Extension( 'a?#b.c' ),
+ IEUrlExtension::findIE6Extension( 'a?#b.c' ),
'Hash character preceding extension 2'
);
}
function testDotAtEnd() {
$this->assertEquals(
'',
- WebRequest::findIE6Extension( '.' ),
+ IEUrlExtension::findIE6Extension( '.' ),
'Dot at end of string'
);
}