-
-
Notifications
You must be signed in to change notification settings - Fork 848
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ACL checks in the media file details ajax calls only for root/arbitrary namespace #765
Comments
ouch. Can you provide a quick fix? |
This should be superseded by a proper rewrite of the media manager code
I noticed this same issue with media manager & acl just today, but from the other way around. When a user has full access to a namespace but no access to the parent namespace. The media manager shows the images but gives a denied access message when viewing details. Was the $NS global 'rationalised' recently? A side question - how can you check the user's permissions to a namespace? medialist plugin does a quick_aclcheck() using the namespace, but gets the actual permissions for the page (named as namespace) in the parent namespace. |
Namespace permissions are checked by adding ':*' to the namespace usually. |
@michitux your fix resolves my issue. |
I will be home in an hour or so and build the hotfix release. |
What about including 59d6be9 in the hotfix as well. This will improve the usability of the extension manager significantly, but is of course not urgent. |
Quick fix for #765 - ACL checks in the media manager ajax calls
This should be superseded by a proper rewrite of the media manager code
This should be superseded by a proper rewrite of the media manager code Conflicts: inc/template.php
Hotfix is out. |
Does that issue only apply to the current release or are all (some) older releases are affected as well? |
It applies to at least the last two releases, maybe even older. |
Will you push a new tag (eg. |
tags pushed. I think I just learned something new about tags :-) |
* stable: (474 commits) hotfix release for #765 Quick fix for #765 - ACL checks in the media manager ajax calls Use git attributes to exclude some files from exported archives Release 2014-05-05 "Ponder Stibbons" Release preparation no fancy quotes in user manager import description add defaults to phpdocs of search universal update deprecation stuff for dw_qearch translation update translation update translation update added another test for arrays fixed some test inheriting from the wrong parent use new $INPUT->valid() method in feed.php add new valid() method to $INPUT #667 some updates on phpunit docs and settings Fix https proxy authentication, the header was missing a colon so that the auth info was not working. translation update translation update translation update ...
A comment in our old bug tracker alerted me of this issue: in the media manager ACLs are broken for all views for individual files if you have access to the root namespace or - in the case of the media diff ajax call - an arbitrary namespace.
I have reproduced this issue in my local DokuWiki installation, I can simply open an image I have no permission for in the media manager, I get a permission denied message in the media details tab and when I click on the detail tabs they load via ajax with the real content. The media diff ajax call is a bit more difficult to test as the ns parameter (but only that one) is a post parameter, but after changing the code to accept a get parameter as well I can clearly see that it uses the ns parameter for the permission check (and nothing else).
No actual file content is exposed, just the metadata, but the metadata can contain a lot of information (title, caption etc. from the exif metadata) and the full history is displayed, too. This also requires knowing the actual media file id.
So far I can see the following problems in the code:
ajax_mediadetails()
,$NS
is not set, but intpl_mediaFileDetails()
$NS
is used and its value is only checked when it is set (if(isset($NS) && getNS($image) != $NS) return;
)$AUTH
is a cache for the permissions of the current media ns (wtf?) which is set byajax_mediadiff()
using the "ns" parameter as namespace without any checks if this is actually the namespace of the current media file. This check is done intpl_mediaFileDetails()
but not inmedia_diff()
which is called inajax_mediadiff()
.I think a first fix could be to ignore the ns parameter in all ajax calls and instead set it based on the supplied image id.
I think we should fix this and release a hotfix release ASAP.
The text was updated successfully, but these errors were encountered: