r55800 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r55799‎ | r55800 | r55801 >
Date:02:44, 4 September 2009
Author:jdpond
Status:reverted (Comments)
Tags:
Comment:
bug 19646 Localization of img_auth.php - with enhancements
https://bugzilla.wikimedia.org/show_bug.cgi?id=19646

1. Localize img_auth.php using core messages
2. Reorder checks to make sense (and eliminate redundancy)n
3. Add hook 'ImgAuthBeforeStream' to allow custom checking
4. Add globals wgImgAuthDetails,
5. Move all "wfDebugLog" into the rejection functions
Modified paths:
  • /trunk/phase3/img_auth.php (modified) (history)
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesEn.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesQqq.php (modified) (history)
  • /trunk/phase3/maintenance/language/messages.inc (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/language/messages.inc
@@ -327,6 +327,17 @@
328328 'anonnotice',
329329 'newsectionheaderdefaultlevel',
330330 'red-link-title',
 331+ 'img-auth-accessdenied',
 332+ 'img-auth-desc',
 333+ 'img-auth-nopathinfo',
 334+ 'img-auth-notindir',
 335+ 'img-auth-badtitle',
 336+ 'img-auth-nologinnWL',
 337+ 'img-auth-nofile',
 338+ 'img-auth-isdir',
 339+ 'img-auth-streaming',
 340+ 'img-auth-public',
 341+ 'img-auth-noread',
331342 ),
332343 'nstab' => array(
333344 'nstab-main',
Index: trunk/phase3/includes/DefaultSettings.php
@@ -191,6 +191,14 @@
192192 $wgFileStore['deleted']['url'] = null; ///< Private
193193 $wgFileStore['deleted']['hash'] = 3; ///< 3-level subdirectory split
194194
 195+
 196+/**
 197+ * used only for img_auth script - see [[Image Authorization]]
 198+ */
 199+$wgImgAuthDetails = false; ///< defaults to false - only set to true if you use img_auth and want the user to see details on why access failed
 200+$wgImgAuthPublicTest = true; ///< defaults to true - if public read is turned on, no need for img_auth, config error unless other access is used
 201+
 202+
195203 /**@{
196204 * File repository structures
197205 *
@@ -281,7 +289,6 @@
282290 */
283291 $wgLegalTitleChars = " %!\"$&'()*,\\-.\\/0-9:;=?@A-Z\\\\^_`a-z~\\x80-\\xFF+";
284292
285 -
286293 /**
287294 * The external URL protocols
288295 */
Index: trunk/phase3/img_auth.php
@@ -3,122 +3,112 @@
44 /**
55 * Image authorisation script
66 *
7 - * To use this:
 7+ * To use this, see http://www.mediawiki.org/wiki/Manual:Image_Authorization
88 *
99 * - Set $wgUploadDirectory to a non-public directory (not web accessible)
1010 * - Set $wgUploadPath to point to this file
1111 *
12 - * Your server needs to support PATH_INFO; CGI-based configurations
13 - * usually don't.
 12+ * Optional Parameters
1413 *
 14+ * - Set $wgImgAuthDetails = true if you want the reason the access was denied messages to be displayed
 15+ * instead of just the 403 error (doesn't work on IE anyway), otherwise will only appear in error logs
 16+ * - Set $wgImgAuthPublicTest false if you don't want to just check and see if all are public
 17+ * must be set to false if using specific restrictions such as LockDown or NSFileRepo
 18+ *
 19+ * For security reasons, you usually don't want your user to know *why* access was denied, just that it was.
 20+ * If you want to change this, you can set $wgImgAuthDetails to 'true' in localsettings.php and it will give the user the reason
 21+ * why access was denied.
 22+ *
 23+ * Your server needs to support PATH_INFO; CGI-based configurations usually don't.
 24+ *
1525 * @file
16 - */
17 -
 26+ *
 27+ **/
 28+
1829 define( 'MW_NO_OUTPUT_COMPRESSION', 1 );
1930 require_once( dirname( __FILE__ ) . '/includes/WebStart.php' );
2031 wfProfileIn( 'img_auth.php' );
2132 require_once( dirname( __FILE__ ) . '/includes/StreamFile.php' );
2233
2334 $perms = User::getGroupPermissions( array( '*' ) );
24 -if ( in_array( 'read', $perms, true ) ) {
25 - wfDebugLog( 'img_auth', 'Public wiki' );
26 - wfPublicError();
27 -}
2835
 36+// See if this is a public Wiki (no protections)
 37+if ( $wgImgAuthPublicTest && in_array( 'read', $perms, true ) )
 38+ wfForbidden('img-auth-accessdenied','img-auth-public');
 39+
2940 // Extract path and image information
30 -if( !isset( $_SERVER['PATH_INFO'] ) ) {
31 - wfDebugLog( 'img_auth', 'Missing PATH_INFO' );
32 - wfForbidden();
33 -}
 41+if( !isset( $_SERVER['PATH_INFO'] ) )
 42+ wfForbidden('img-auth-accessdenied','img-auth-nopathinfo');
3443
3544 $path = $_SERVER['PATH_INFO'];
3645 $filename = realpath( $wgUploadDirectory . $_SERVER['PATH_INFO'] );
3746 $realUpload = realpath( $wgUploadDirectory );
38 -wfDebugLog( 'img_auth', "\$path is {$path}" );
39 -wfDebugLog( 'img_auth', "\$filename is {$filename}" );
4047
4148 // Basic directory traversal check
42 -if( substr( $filename, 0, strlen( $realUpload ) ) != $realUpload ) {
43 - wfDebugLog( 'img_auth', 'Requested path not in upload directory' );
44 - wfForbidden();
45 -}
 49+if( substr( $filename, 0, strlen( $realUpload ) ) != $realUpload )
 50+ wfForbidden('img-auth-accessdenied','img-auth-notindir');
4651
4752 // Extract the file name and chop off the size specifier
4853 // (e.g. 120px-Foo.png => Foo.png)
4954 $name = wfBaseName( $path );
5055 if( preg_match( '!\d+px-(.*)!i', $name, $m ) )
5156 $name = $m[1];
52 -wfDebugLog( 'img_auth', "\$name is {$name}" );
5357
 58+// Check to see if the file exists
 59+if( !file_exists( $filename ) )
 60+ wfForbidden('img-auth-accessdenied','img-auth-nofile',htmlspecialchars($filename));
 61+
 62+// Check to see if tried to access a directory
 63+if( is_dir( $filename ) )
 64+ wfForbidden('img-auth-accessdenied','img-auth-isdir',htmlspecialchars($filename));
 65+
 66+
5467 $title = Title::makeTitleSafe( NS_FILE, $name );
55 -if( !$title instanceof Title ) {
56 - wfDebugLog( 'img_auth', "Unable to construct a valid Title from `{$name}`" );
57 - wfForbidden();
58 -}
59 -if( !$title->userCanRead() ) {
60 - wfDebugLog( 'img_auth', "User does not have access to read `{$name}`" );
61 - wfForbidden();
62 -}
63 -$title = $title->getPrefixedText();
6468
65 -// Check the whitelist if needed
66 -if( !$wgUser->getId() && ( !is_array( $wgWhitelistRead ) || !in_array( $title, $wgWhitelistRead ) ) ) {
67 - wfDebugLog( 'img_auth', "Not logged in and `{$title}` not in whitelist." );
68 - wfForbidden();
69 -}
 69+// See if could create the title object
 70+if( !$title instanceof Title )
 71+ wfForbidden('img-auth-accessdenied','img-auth-badtitle',htmlspecialchars($name));
7072
71 -if( !file_exists( $filename ) ) {
72 - wfDebugLog( 'img_auth', "`{$filename}` does not exist" );
73 - wfForbidden();
74 -}
75 -if( is_dir( $filename ) ) {
76 - wfDebugLog( 'img_auth', "`{$filename}` is a directory" );
77 - wfForbidden();
78 -}
 73+// Run hook
 74+if (!wfRunHooks( 'ImgAuthBeforeStream', array( &$title, &$path, &$name, &$result ) ) )
 75+ call_user_func_array('wfForbidden',merge_array(array($result[0],$result[1]),array_slice($result,2)));
 76+
 77+// Check user authorization for this title
 78+// UserCanRead Checks Whitelist too
 79+if( !$title->userCanRead() )
 80+ wfForbidden('img-auth-accessdenied','img-auth-noread',htmlspecialchars($name));
7981
 82+
8083 // Stream the requested file
81 -wfDebugLog( 'img_auth', "Streaming `{$filename}`" );
 84+wfDebugLog( 'img_auth', "Streaming `".htmlspecialchars($filename)."`." );
8285 wfStreamFile( $filename, array( 'Cache-Control: private', 'Vary: Cookie' ) );
8386 wfLogProfilingData();
8487
8588 /**
86 - * Issue a standard HTTP 403 Forbidden header and a basic
87 - * error message, then end the script
 89+ * Issue a standard HTTP 403 Forbidden header ($msg1-a message index, not a message) and an
 90+ * error message ($msg2, also a message index), (both required) then end the script
 91+ * subsequent arguments to $msg2 will be passed as parameters only for replacing in $msg2
8892 */
89 -function wfForbidden() {
 93+function wfForbidden($msg1,$msg2) {
 94+ global $wgImgAuthDetails;
 95+ $args = func_get_args();
 96+ array_shift( $args );
 97+ array_shift( $args );
 98+ $MsgHdr = wfMsgHTML($msg1);
 99+ $detailMsg = call_user_func_array('wfMsgHTML',array_merge(array($wgImgAuthDetails ? $msg2 : 'badaccess-group0'),$args));
 100+ wfDebugLog('img_auth', "wfForbidden Hdr:".wfMsgExt( $msg1, array('language' => 'en'))." Msg: ".
 101+ call_user_func_array('wfMsgExt',array_merge( array($msg2, array('language' => 'en')),$args)));
90102 header( 'HTTP/1.0 403 Forbidden' );
91 - header( 'Vary: Cookie' );
 103+ header( 'Cache-Control: no-cache' );
92104 header( 'Content-Type: text/html; charset=utf-8' );
93105 echo <<<ENDS
94106 <html>
95107 <body>
96 -<h1>Access Denied</h1>
97 -<p>You need to log in to access files on this server.</p>
 108+<h1>$MsgHdr</h1>
 109+<p>$detailMsg</p>
98110 </body>
99111 </html>
100112 ENDS;
101113 wfLogProfilingData();
102114 exit();
103115 }
104 -
105 -/**
106 - * Show a 403 error for use when the wiki is public
107 - */
108 -function wfPublicError() {
109 - header( 'HTTP/1.0 403 Forbidden' );
110 - header( 'Content-Type: text/html; charset=utf-8' );
111 - echo <<<ENDS
112 -<html>
113 -<body>
114 -<h1>Access Denied</h1>
115 -<p>The function of img_auth.php is to output files from a private wiki. This wiki
116 -is configured as a public wiki. For optimal security, img_auth.php is disabled in
117 -this case.
118 -</p>
119 -</body>
120 -</html>
121 -ENDS;
122 - wfLogProfilingData();
123 - exit;
124 -}
125 -
Index: trunk/phase3/languages/messages/MessagesQqq.php
@@ -2641,6 +2641,19 @@
26422642 * $1 is a date (example: ''19 March 2008'')
26432643 * $2 is a time (example: ''12:15'')",
26442644
 2645+#img_auth script messages
 2646+'img-auth-desc' => '[[Manual:Image Authorization]] script, see http://www.mediawiki.org/wiki/Manual:Image_Authorization',
 2647+'img-auth-accessdenied' => "[[Manual:Image Authorization]] Access Denied",
 2648+'img-auth-nopathinfo' => "[[Manual:Image Authorization]] Missing PATH_INFO - see english description",
 2649+'img-auth-notindir' => "[[Manual:Image Authorization]] when the specified path is not in upload directory.",
 2650+'img-auth-badtitle' => "[[Manual:Image Authorization]] bad title, $1 is the invalid title",
 2651+'img-auth-nologinnWL' => "[[Manual:Image Authorization]] logged in and file not whitelisted. $1 is the file not in whitelist.",
 2652+'img-auth-nofile' => "[[Manual:Image Authorization]] non existent file, $1 is the file that does not exist.",
 2653+'img-auth-isdir' => "[[Manual:Image Authorization]] trying to access a directory instead of a file, $1 is the directory.",
 2654+'img-auth-streaming' => "[[Manual:Image Authorization]] is now streaming file specified by $1.",
 2655+'img-auth-public' => "[[Manual:Image Authorization]] an error message when the admin has configured the wiki to be a public wiki, but is using img_auth script - normally this is a configuration error, except when special restriction extensions are used",
 2656+'img-auth-noread' => "[[Manual:Image Authorization]] User does not have access to read file, $1 is the file",
 2657+
26452658 # Video information, used by Language::formatTimePeriod() to format lengths in the above messages
26462659 'seconds-abbrev' => '{{optional}}',
26472660 'minutes-abbrev' => '{{optional}}',
Index: trunk/phase3/languages/messages/MessagesEn.php
@@ -2144,6 +2144,19 @@
21452145 'upload-unknown-size' => 'Unknown size',
21462146 'upload-http-error' => 'An HTTP error occured: $1',
21472147
 2148+#img_auth script messages
 2149+'img-auth-accessdenied' => "Access Denied",
 2150+'img-auth-desc' => 'Image authorisation script',
 2151+'img-auth-nopathinfo' => "Missing PATH_INFO. Your server is not set up to pass this information - may be CGI-based and can't support img_auth. See http://www.mediawiki.org/wiki/Manual:Image_Authorization.",
 2152+'img-auth-notindir' => "Requested path is not in the configured upload directory.",
 2153+'img-auth-badtitle' => "Unable to construct a valid title from `$1`.",
 2154+'img-auth-nologinnWL' => "You are not logged in and `$1` not in whitelist.",
 2155+'img-auth-nofile' => "File `$1` does not exist.",
 2156+'img-auth-isdir' => "you are trying to access a directory`$1`. Only file access is allowed.",
 2157+'img-auth-streaming' => "Streaming `$1`.",
 2158+'img-auth-public' => "The function of img_auth.php is to output files from a private wiki. This wiki is configured as a public wiki. For optimal security, img_auth.php is disabled for this case.",
 2159+'img-auth-noread' => "User does not have access to read `$1`.",
 2160+
21482161 # Some likely curl errors. More could be added from <http://curl.haxx.se/libcurl/c/libcurl-errors.html>
21492162 'upload-curl-error6' => 'Could not reach URL',
21502163 'upload-curl-error6-text' => 'The URL provided could not be reached.

Follow-up revisions

RevisionCommit summaryAuthorDate
r55804Follow-up to r55800...siebrand06:19, 4 September 2009
r55806Use consistent quotes in new messages (r55800)siebrand06:44, 4 September 2009
r55808Remove unused message 'img-auth-desc' (r55800)siebrand06:55, 4 September 2009
r55809Tweak 'img-auth-nologinnWL' (r55800)siebrand06:59, 4 September 2009
r56051Revert r55800 "bug 19646 Localization of img_auth.php - with enhancements"...brion18:07, 8 September 2009

Comments

#Comment by Brion VIBBER (talk | contribs)   23:18, 4 September 2009

+wfDebugLog( 'img_auth', "Streaming `".htmlspecialchars($filename)."`." );

^ this escaping is wrong


+ call_user_func_array('wfForbidden',merge_array(array($result[0],$result[1]),array_slice($result,2)));

This is ... hideous. :D Can we non-scary code here?

Status & tagging log