Last modified: 2010-05-15 15:48:20 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.
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() ) );
When would this possibly come up? When Special:Export is whitelisted?
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.
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.
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?
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.
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.
MediaWiki is open-access by design; Special:Export and other tools must not be whitelisted on a closed-access wiki.
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.
I give my reluctant blessing to this experiment. *waves hands mystically*
Where are we going, and what's with the handbasket?
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.
