From: Tim Starling <tstarling@users.mediawiki.org>
Date: Wed, 1 Jun 2011 02:01:59 +0000 (+0000)
Subject: * Only blacklist query string extensions which match /^[a-zA-Z0-9_-]+$/. This avoids... 
X-Git-Tag: 1.31.0-rc.0~29813
X-Git-Url: http://git.cyclocoop.org/%22%20.%20generer_url_ecrire%28%22sites_tous%22%29%20.%20%22?a=commitdiff_plain;h=a9b9efecb45676e95ae6e7e6d9c929a6d18b6ae3;p=lhc%2Fweb%2Fwiklou.git

* Only blacklist query string extensions which match /^[a-zA-Z0-9_-]+$/. This avoids blacklisting pretty much every api.php URL with a dot in it, due to extensions like "webm&smaxage=3600&maxage=3600&format=jsonfm" being detected. Such an extension is unlikely to be registered to a dangerous file type. The proposed regex matches all extensions registered in HKEY_CLASSES_ROOT on my Windows XP VM, but does not include the ampersand, so avoids matching multiple URL parameters.
* Fixed a logic error in WebRequest::isPathInfoBad() from r88883, which caused dangerous PATH_INFO strings to be allowed as long as QUERY_STRING was set.
* Refactored the query string checks in WebRequest and img_auth.php into a single new function: isQueryStringBad().
---

diff --git a/img_auth.php b/img_auth.php
index 5ac56999f5..ac3f50e93c 100644
--- a/img_auth.php
+++ b/img_auth.php
@@ -43,8 +43,7 @@ if ( $wgImgAuthPublicTest
 }
 
 // Check for bug 28235: QUERY_STRING overriding the correct extension
-if ( isset( $_SERVER['QUERY_STRING'] )
-	&& preg_match( '/\.[^\\/:*?"<>|%]+(#|\?|$)/i', $_SERVER['QUERY_STRING'] ) )
+if ( $wgRequest->isQueryStringBad() )
 {
 	wfForbidden( 'img-auth-accessdenied', 'img-auth-bad-query-string' );
 }	
diff --git a/includes/WebRequest.php b/includes/WebRequest.php
index 1e0b98d832..f40d54a50f 100644
--- a/includes/WebRequest.php
+++ b/includes/WebRequest.php
@@ -785,10 +785,8 @@ class WebRequest {
 	public function isPathInfoBad() {
 		global $wgScriptExtension;
 
-		if ( isset( $_SERVER['QUERY_STRING'] ) )
-		{
-			// Bug 28235
-			return strval( self::findIE6Extension( $_SERVER['QUERY_STRING'] ) ) !== '';
+		if ( $this->isQueryStringBad() ) {
+			return true;
 		}
 
 		if ( !isset( $_SERVER['PATH_INFO'] ) ) {
@@ -875,6 +873,24 @@ class WebRequest {
 		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.
+	 */
+	public function isQueryStringBad() {
+		if ( !isset( $_SERVER['QUERY_STRING'] ) ) {
+			return false;
+		}
+
+		$extension = self::findIE6Extension( $_SERVER['QUERY_STRING'] );
+		if ( $extension === '' ) {
+			return false;
+		}
+
+		return (bool)preg_match( '/^[a-zA-Z0-9_-]+$/', $extension );
+	}
+
 	/**
 	 * Parse the Accept-Language header sent by the client into an array
 	 * @return array( languageCode => q-value ) sorted by q-value in descending order