Last modified: 2010-05-15 15:48:20 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 8824 - Only export pages the user can read
Only export pages the user can read
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
Special pages (Other open bugs)
1.9.x
PC Windows XP
: Normal normal (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-29 18:28 UTC by Fernando Correia
Modified: 2010-05-15 15:48 UTC (History)
0 users

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
does not export pages the user can't read (436 bytes, patch)
2007-01-29 18:29 UTC, Fernando Correia
Details

Description Fernando Correia 2007-01-29 18:28:36 UTC
Special:Export doesn't check whether the user has the rights to read a page that
is being exported. This can be used to get access to pages that should not be
available to that user.

The proposed patch verifies if the user can read the page and does not export
pages that are not allowed to that user.
Comment 1 Fernando Correia 2007-01-29 18:29:55 UTC
Created attachment 3156 [details]
does not export pages the user can't read

Index: Export.php
===================================================================
--- Export.php	(revision 19680)
+++ Export.php	(working copy)
@@ -110,6 +110,9 @@
	 * @param Title $title
	 */
	function pageByTitle( $title ) {
+		if (!$title->userCanRead()) {
+			return;
+		}
		return $this->dumpFrom(
			'page_namespace=' . $title->getNamespace() .
			' AND page_title=' . $this->db->addQuotes(
$title->getDbKey() ) );
Comment 2 Aryeh Gregor (not reading bugmail, please e-mail directly) 2007-01-30 02:22:39 UTC
When would this possibly come up?  When Special:Export is whitelisted?
Comment 3 Fernando Correia 2007-01-30 13:26:41 UTC
This would come up when the userCan hook was used to prevent read access to some
pages according to the groups the user belongs to.
Comment 4 Daniel Kinzler 2007-01-30 13:34:02 UTC
while i think it would indeed be good to check access when exporting, the
simplest workaround would indded be to just deny ordinary users access to
Special:Export.
Comment 5 Fernando Correia 2007-01-30 16:02:47 UTC
I understand. But let's assume someone wants to hook userCan so some users can
see some pages, other users can see other pages, and it is still desired that
the users can export the pages they have read access to.

This patch would really help to close a breach that troubles security
extensions. If there is a problem with the patch I'm willing to work it out. Do
you think a hook would be better?
Comment 6 Aryeh Gregor (not reading bugmail, please e-mail directly) 2007-01-30 16:54:00 UTC
Well, I don't see any harm in this patch, I guess, but realize that MediaWiki is not configured 
for granular read security and if you try to add it you'll have to plug a million holes like this.

A hook is probably a better idea.
Comment 7 Fernando Correia 2007-01-30 17:11:59 UTC
You're right, but each hole that is plugged is one less hole. Actually it
doesn't seem there are so many holes. The code base already calls "userCanRead"
and "userCanEdit" in a lot of places.

I suggest we try to extend its use to a few additional places such as the page
export.

Someone would have to decide between calling userCanRead and calling a hook that
will either do nothing (if not defined) or call userCanRead. I think the first
option is more clear and more consistent with the rest of the code base. Only if
there is a significant performance impact I would substitute it for a hook.
Comment 8 Brion Vibber 2007-02-02 18:55:29 UTC
MediaWiki is open-access by design; Special:Export and other tools must not be
whitelisted on a closed-access wiki.
Comment 9 Daniel Kinzler 2007-02-15 01:28:50 UTC
Fixed in r19935. Beware however that other loopholes to read-restrictions exist
(such as including as a template, which I addressed in r19934). Mediawiki will
probably not support air-tight read-restrictions any time soon. But we *can*
plug the large and easy holes, I guess. 
Comment 10 Brion Vibber 2007-02-15 01:38:29 UTC
I give my reluctant blessing to this experiment.

*waves hands mystically*
Comment 11 Rob Church 2007-02-15 08:03:23 UTC
Where are we going, and what's with the handbasket?
Comment 12 Fernando Correia 2007-02-15 09:57:27 UTC
That is great news. Thank you all! I agree that MediaWiki core doesn't need to
implement advanced access control. Extensions may try and handle that. All I ask
is some hooks and basic checks like this one to make these extensions simpler
and more reliable.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links