?
avatar PhilETaylor
PhilETaylor
2 Feb 2017

Steps to reproduce the issue

install 3.7.0b1
create Media custom field
Set the fields params to HOME DIRECTORY = Yes
Dont set anything else
edit content on FRONTEND

Expected result

That I, as username "admin" cannot see other users folders

Actual result

I can manipulate the url of the popup media manager (or just click the up button) to view other folders outside of the "home" folder /images/images/admin/

avatar PhilETaylor PhilETaylor - open - 2 Feb 2017
avatar joomla-cms-bot joomla-cms-bot - change - 2 Feb 2017
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - labeled - 2 Feb 2017
avatar Bakual
Bakual - comment - 2 Feb 2017

Is this related to #13582 ?

avatar PhilETaylor
PhilETaylor - comment - 2 Feb 2017

No.

This is about the fact that although the field is configured to lock a user to his home directory (whereever that is, and whatever it is named, see the advice in that PR) - that a user (with permissions) can still manipulate and get out of HIS home folder.

What I see a lot of in the new com_fields is that the validation of the various options is not correctly enforced

This is a security issue.

avatar laoneo
laoneo - comment - 3 Feb 2017

This is a limitation of the media manager form field. This flag is more for the initial directory and not to lock the user to a specific directory. We have planed that feature in the "New Media Manager" project for a future release.

avatar PhilETaylor
PhilETaylor - comment - 3 Feb 2017

This flag is more for the initial directory and not to lock the user to a specific directory.

THEN THIS NEEDS HUGE EXPLAINING TO THE SITE ADMIN because else this is a security issue allowing users to navigate to ANYWHERE in the /images folder

I dont give a damn about "New Media Manager" - once that code is merged then we can talk about that - for today, this is Joomla 3.7.0 in beta 1 and should NOT have such a security issue released!

avatar beat
beat - comment - 3 Feb 2017

I agree with @PhilETaylor that this is a security issue. Normal users in frontend or non-super-admin users should never have access to all server or to all site files.

avatar mbabker
mbabker - comment - 3 Feb 2017

There isn't a way to fix this without doing some deep level intercoupling between com_media and *something* (what I don't know). As everyone knows, you can craft URLs in a way to view any directory within your configured images path. So you can't use URL parameters as a means for access restriction, easily removed. Same goes for identifying the request context as something for a field with a configured ACL limitation. So data would have to be set by Joomla somewhere (session, user state (so still session)) to know what the user can or cannot do when browsing media through this component with a dynamic context.

I hate to sound like I'm agreeing with the "can't fix" argument right now, but without a proper ACL implementation in com_media (a feature on the new media manager), I don't see how this can be adequately fixed.

avatar PhilETaylor
PhilETaylor - comment - 3 Feb 2017

So again - architectural failure, bad implementation of a new feature now allows directory traversal and the project is happy with that...

#fail

Sent from my iPhone

On 3 Feb 2017, at 13:58, Michael Babker notifications@github.com wrote:

There isn't a way to fix this without doing some deep level intercoupling between com_media and something (what I don't know). As everyone knows, you can craft URLs in a way to view any directory within your configured images path. So you can't use URL parameters as a means for access restriction, easily removed. Same goes for identifying the request context as something for a field with a configured ACL limitation. So data would have to be set by Joomla somewhere (session, user state (so still session)) to know what the user can or cannot do when browsing media through this component with a dynamic context.

I hate to sound like I'm agreeing with the "can't fix" argument right now, but without a proper ACL implementation in com_media (a feature on the new media manager), I don't see how this can be adequately fixed.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

avatar mbabker
mbabker - comment - 3 Feb 2017

Navigate to any Joomla site's /images path on a server where directory listings aren't disabled. Directory traversal. That's outside the scope of anything Joomla can address, and that's the exact directory path we're discussing here. All of this content is already in an insecure location within the web root since it is publicly accessible if you know the right URLs to craft.

Not saying the current situation is acceptable, but we aren't talking about stuff that is already secure and this new option is exposing a new security hole.

avatar PhilETaylor
PhilETaylor - comment - 3 Feb 2017

My point is that if you set the field to be (assuming that's a restriction) to a users home directory when configuring the field the EXPECTATION of the admin is that the user can then only see/upload to THAT FOLDER and below

The EXPECTATION is that the user will be limited to that folder and not see other users home folders

Again, if you think it's acceptable for an admin to set a home directory and then acceptable to allow the end user to ignore that setting completely then what's the point of having that setting at all!

Sent from my iPhone

On 3 Feb 2017, at 14:04, Michael Babker notifications@github.com wrote:

Navigate to any Joomla site's /images path on a server where directory listings aren't disabled. Directory traversal. That's outside the scope of anything Joomla can address, and that's the exact directory path we're discussing here. All of this content is already in an insecure location within the web root since it is publicly accessible if you know the right URLs to craft.

Not saying the current situation is acceptable, but we aren't talking about stuff that is already secure and this new option is exposing a new security hole.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

avatar mbabker
mbabker - comment - 3 Feb 2017

I'm not saying the current behavior is right by any means. I hope nobody is to be honest. But:

  1. com_media does not support any notion of ACL, this is the primary issue that needs addressing and as I noted its implementation can't be reliant on URL parameters passed when using the modal view; this isn't a simple fix as it basically means your media assets have to have corresponding records in the database for JAccess to correctly function
  2. The path we are discussing is already publicly accessible and depending on server configuration exposes all data. Nothing there can be considered safe or secure. If you're really looking for that, find a file management extension which stores its uploads outside the web directory, accesses the files through a request to Joomla (and good luck getting Joomla to serve non-HTML content without FUBAR'ing everything), and write all the integrations needed to replace com_media.
avatar laoneo
laoneo - comment - 3 Feb 2017

As @mbabker explained it is just not possible to implement that feature in a 100% safe way. Otherwise we have to obfuscate paths and filenames of the files, and everybody knows where this path is leading to.
So @mbabker, @PhilETaylor and @beat what you guys think, should we remove then that option?

avatar PhilETaylor
PhilETaylor - comment - 3 Feb 2017

It's perfectly simple/reasonable to restrict the path in the session

However the first question to answer is why on earth there is a home directory feature of the media field if users are - by design- allows to navigate anywhere

I don't think enough thought has been given to the use of the options provided with most fields - this is a good example - in real world use, this feature if the media field is not implemented in a way that makes the feature useful

Sent from my iPhone

On 3 Feb 2017, at 14:37, Allon Moritz notifications@github.com wrote:

As @mbabker explained it is just not possible to implement that feature in a 100% safe way. Otherwise we have to obfuscate paths and filenames of the files, and everybody knows where this path is leading to.
So @mbabker, @PhilETaylor and @beat what you guys think, should we remove then that option?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

avatar laoneo
laoneo - comment - 3 Feb 2017

A quick and dirty solution would be to just hide the directory traversable select box. But it is still not 100% secure.

avatar mbabker
mbabker - comment - 3 Feb 2017

I would suggest looking for a way to hide the "up" button (I assume this is where the main contention lies) so at least it gets a little harder for someone to traverse the paths as a starting point. How that's done I wouldn't be able to suggest at this point but it would probably cut back on the number of reports similar to this one both here and in the JSST mailbox. The other option I guess would be to make it more clear in the UI what exactly is happening with this option so we can point to something (tooltip, documentation, whatever) about why the behavior is the way it is versus the expectation of a fully implemented ACL system.

avatar laoneo
laoneo - comment - 3 Feb 2017

If this feature as it is now, is treated as a security whole, then I suggest we are going to remove it. As you guys said already it needs a proper ACL set up behind all of this to implement it 100% secure and that's IMO the job of a document management system and not the media manager.

avatar Bakual
Bakual - comment - 3 Feb 2017

Or just rename it from Home to Default or something else which indicates it's not a restricting/security option.

avatar laoneo
laoneo - comment - 3 Feb 2017

Guess an admin would not understand what Default means.

avatar laoneo
laoneo - comment - 3 Feb 2017

Opened a PR #13893 to remove the home option.

avatar beat
beat - comment - 3 Feb 2017

I think the home option is a useful setting to help keep a site ordered (with reasonable users having author level).

Maybe just renaming it or adding a warning around the setting like ?:
"Home (default folder, WARNING: User still has unrestricted access to all files)"

Removing that option won't make security better. But renaming its title and description would clarify that it is not an additional security feature. And most importantly clarify the access given to user anyway.

avatar dgt41
dgt41 - comment - 3 Feb 2017

https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_media/media.php#L41-L42
Define the root folder for the folder to display, so enhancing this to also check for the input folder (&folder=images/username) should fix the problem

avatar laoneo
laoneo - comment - 3 Feb 2017

I guess you don't want to do that site wide, just for the custom field. I mean mainly this makes sense only for users, for example for the avatar use case. Otherwise we have the situation that one user is selecting an image for an article and then the other user is changing to a different one, because the entry is empty.

avatar dgt41
dgt41 - comment - 3 Feb 2017

if we want to introduce locked-in behaviour somehow we need to control it. Obviously there are more aspects, like the one you're referring, that also need to keep working. I guess there is no 1-line solution here

avatar justinherrin
justinherrin - comment - 3 Feb 2017

From an "outsiders" perspective here, I would recommend that the Media Field type not be included in Joomla 3.7.x until the "New Media Manager" with complete ACL features is merged into the project. Once that happens, then you can release the Media Field type to the world to use.

Just my two cents.

avatar zero-24
zero-24 - comment - 23 Feb 2017

Closing as the feature is removed from the core now.

avatar zero-24 zero-24 - change - 23 Feb 2017
Status New Closed
Closed_Date 0000-00-00 00:00:00 2017-02-23 13:16:26
Closed_By zero-24
avatar zero-24 zero-24 - close - 23 Feb 2017

Add a Comment

Login with GitHub to post a comment