r85848 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r85847‎ | r85848 | r85849 >
Date:01:23, 12 April 2011
Author:tstarling
Status:ok
Tags:
Comment:
MFT r85844 via REL1_17 85846: IE 6 XSS fix
Modified paths:
  • /branches/wmf/1.17wmf1/api.php (modified) (history)
  • /branches/wmf/1.17wmf1/img_auth.php (modified) (history)
  • /branches/wmf/1.17wmf1/includes/RawPage.php (modified) (history)
  • /branches/wmf/1.17wmf1/includes/WebRequest.php (modified) (history)
  • /branches/wmf/1.17wmf1/languages/messages/MessagesEn.php (modified) (history)
  • /branches/wmf/1.17wmf1/load.php (modified) (history)

Diff [purge]

Index: branches/wmf/1.17wmf1/includes/WebRequest.php
——@@ -746,10 +746,27 @@
747747 * but only by prefixing it with the script name and maybe some other stuff,
748748 * the extension is not mangled. So this should be a reasonably portable
749749 * way to perform this security check.
 750+ *
 751+ * Also checks for anything that looks like a file extension at the end of
 752+ * QUERY_STRING, since IE 6 and earlier will use this to get the file type
 753+ * if there was no dot before the question mark (bug 28235).
750754 */
751755 public function isPathInfoBad() {
752756 global $wgScriptExtension;
753757
 758+ if ( isset( $_SERVER['QUERY_STRING'] )
 759+ && preg_match( '/\.[a-z]{1,4}$/i', $_SERVER['QUERY_STRING'] ) )
 760+ {
 761+ // Bug 28235
 762+ // Block only Internet Explorer 6, and requests with missing UA
 763+ // headers that could be IE users behind a privacy proxy.
 764+ if ( !isset( $_SERVER['HTTP_USER_AGENT'] )
 765+ || preg_match( '/; *MSIE 6/', $_SERVER['HTTP_USER_AGENT'] ) )
 766+ {
 767+ return true;
 768+ }
 769+ }
 770+
754771 if ( !isset( $_SERVER['PATH_INFO'] ) ) {
755772 return false;
756773 }
Property changes on: branches/wmf/1.17wmf1/includes/WebRequest.php
___________________________________________________________________
Added: svn:mergeinfo
757774 Merged /branches/REL1_15/phase3/includes/WebRequest.php:r51646
758775 Merged /branches/wmf/1.16wmf4/includes/WebRequest.php:r67177,69199,76243,77266
759776 Merged /branches/REL1_17/phase3/includes/WebRequest.php:r85846
760777 Merged /branches/sqlite/includes/WebRequest.php:r58211-58321
761778 Merged /trunk/phase3/includes/WebRequest.php:r83590
762779 Merged /branches/REL1_17/phase3/images/includes/WebRequest.php:r85846
763780 Merged /branches/new-installer/phase3/includes/WebRequest.php:r43664-66004
764781 Merged /branches/wmf-deployment/includes/WebRequest.php:r53381,60970
Index: branches/wmf/1.17wmf1/includes/RawPage.php
——@@ -132,7 +132,7 @@
133133 #
134134 # Just return a 403 Forbidden and get it over with.
135135 wfHttpError( 403, 'Forbidden',
136 - 'Invalid file extension found in PATH_INFO. ' .
 136+ 'Invalid file extension found in PATH_INFO or QUERY_STRING. ' .
137137 'Raw pages must be accessed through the primary script entry point.' );
138138 return;
139139 }
Property changes on: branches/wmf/1.17wmf1/includes/RawPage.php
___________________________________________________________________
Added: svn:mergeinfo
140140 Merged /branches/REL1_15/phase3/includes/RawPage.php:r51646
141141 Merged /branches/wmf/1.16wmf4/includes/RawPage.php:r67177,69199,76243,77266
142142 Merged /branches/REL1_17/phase3/includes/RawPage.php:r85846
143143 Merged /branches/sqlite/includes/RawPage.php:r58211-58321
144144 Merged /trunk/phase3/includes/RawPage.php:r83590
145145 Merged /branches/REL1_17/phase3/images/includes/RawPage.php:r85846
146146 Merged /branches/new-installer/phase3/includes/RawPage.php:r43664-66004
147147 Merged /branches/wmf-deployment/includes/RawPage.php:r53381,60970
Index: branches/wmf/1.17wmf1/img_auth.php
——@@ -37,6 +37,13 @@
3838 wfForbidden('img-auth-accessdenied','img-auth-public');
3939 }
4040
 41+// Check for bug 28235: QUERY_STRING overriding the correct extension
 42+if ( isset( $_SERVER['QUERY_STRING'] )
 43+ && preg_match( '/\.[a-z]{1,4}$/i', $_SERVER['QUERY_STRING'] ) )
 44+{
 45+ wfForbidden( 'img-auth-accessdenied', 'img-auth-bad-query-string' );
 46+}
 47+
4148 // Extract path and image information
4249 if( !isset( $_SERVER['PATH_INFO'] ) ) {
4350 $path = $wgRequest->getText( 'path' );
Property changes on: branches/wmf/1.17wmf1/img_auth.php
___________________________________________________________________
Modified: svn:mergeinfo
4451 Merged /branches/REL1_16/phase3/img_auth.php:r85845
4552 Merged /branches/REL1_17/phase3/img_auth.php:r85846
4653 Merged /branches/REL1_17/phase3/images/img_auth.php:r85846
Index: branches/wmf/1.17wmf1/api.php
——@@ -55,8 +55,7 @@
5656 //
5757 if ( $wgRequest->isPathInfoBad() ) {
5858 wfHttpError( 403, 'Forbidden',
59 - 'Invalid file extension found in PATH_INFO. ' .
60 - 'The API must be accessed through the primary script entry point.' );
 59+ 'Invalid file extension found in PATH_INFO or QUERY_STRING.' );
6160 return;
6261 }
6362
Property changes on: branches/wmf/1.17wmf1/api.php
___________________________________________________________________
Added: svn:mergeinfo
6463 Merged /branches/wmf/1.16wmf4/api.php:r67177,69199,76243,77266
6564 Merged /branches/REL1_17/phase3/api.php:r85846
6665 Merged /branches/sqlite/api.php:r58211-58321
6766 Merged /trunk/phase3/api.php:r79828,79830,79848,79853,79950-79951,79954,79989,80006-80007,80013,80016,80080,80083,80124,80128,80238,81833,83212,83590
6867 Merged /branches/REL1_17/phase3/images/api.php:r85846
6968 Merged /branches/new-installer/phase3/api.php:r43664-66004
7069 Merged /branches/wmf-deployment/api.php:r60970
7170 Merged /branches/REL1_15/phase3/api.php:r51646
Index: branches/wmf/1.17wmf1/load.php
——@@ -37,11 +37,8 @@
3838 //
3939 if ( $wgRequest->isPathInfoBad() ) {
4040 wfHttpError( 403, 'Forbidden',
41 - 'Invalid file extension found in PATH_INFO. ' .
42 - 'The resource loader must be accessed through the primary script entry point.' );
 41+ 'Invalid file extension found in PATH_INFO or QUERY_STRING.' );
4342 return;
44 - // FIXME: Doesn't this execute the rest of the request anyway?
45 - // Was taken from api.php so I guess it's maybe OK but it doesn't look good.
4643 }
4744
4845 // Respond to resource loading request
Property changes on: branches/wmf/1.17wmf1/load.php
___________________________________________________________________
Added: svn:mergeinfo
4946 Merged /branches/new-installer/phase3/load.php:r43664-66004
5047 Merged /branches/wmf-deployment/load.php:r60970
5148 Merged /branches/REL1_15/phase3/load.php:r51646
5249 Merged /branches/wmf/1.16wmf4/load.php:r67177,69199,76243,77266
5350 Merged /branches/REL1_17/phase3/load.php:r85846
5451 Merged /branches/sqlite/load.php:r58211-58321
5552 Merged /trunk/phase3/load.php:r79828,79830,79848,79853,79950-79951,79954,79989,80006-80007,80013,80016,80080,80083,80124,80128,80238,81833,83212,83590
5653 Merged /branches/REL1_17/phase3/images/load.php:r85846
Index: branches/wmf/1.17wmf1/languages/messages/MessagesEn.php
——@@ -2211,6 +2211,7 @@
22122212 This wiki is configured as a public wiki.
22132213 For optimal security, img_auth.php is disabled.',
22142214 'img-auth-noread' => 'User does not have access to read "$1".',
 2215+'img-auth-bad-query-string' => 'The URL has an invalid query string.',
22152216
22162217 # HTTP errors
22172218 'http-invalid-url' => 'Invalid URL: $1',
Property changes on: branches/wmf/1.17wmf1/languages/messages/MessagesEn.php
___________________________________________________________________
Added: svn:mergeinfo
22182219 Merged /branches/sqlite/languages/messages/MessagesEn.php:r58211-58321
22192220 Merged /trunk/phase3/languages/messages/MessagesEn.php:r79828,79830,79848,79853,79950-79951,79954,79989,80006-80007,80013,80016,80080,80083,80124,80128,80238,83212
22202221 Merged /branches/REL1_17/phase3/images/languages/messages/MessagesEn.php:r85846
22212222 Merged /branches/new-installer/phase3/languages/messages/MessagesEn.php:r43664-66004
22222223 Merged /branches/wmf-deployment/languages/messages/MessagesEn.php:r60970
22232224 Merged /branches/REL1_15/phase3/languages/messages/MessagesEn.php:r51646
22242225 Merged /branches/wmf/1.16wmf4/languages/messages/MessagesEn.php:r67177,69199,76243,77266
22252226 Merged /branches/REL1_17/phase3/languages/messages/MessagesEn.php:r81465,85846

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r85844Fix for bug 28235: IE6 looks for the file extension in the query stringtstarling00:55, 12 April 2011

Status & tagging log