Sacyrity problem! directory listing. about django-filebrowser-no-grappelli

[wardi]

Hmm no directory listing here but I can cause an unrelated error, which is worrying! Do you know if there is a fix upstream?

[pahaz]

Sorry, i can`t fix.
But i can show how fix.
Use this function! Then return absolute path if no Access error.

def _fix_listing(root, *path):
   path = os.path.join(root, *path)
   abspath = os.path.abspath(os.path.normpath(path))
   if not abspath.startswith(root):
       raise ValueError('Access Denited')
   return abspath

>>> _fix_listing("C:\\", "../../", "home")
'C:\\home'
>>> _fix_listing("C:\\dir", "../../", "home")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 5, in _fix_listing
ValueError: Access Denited
>>> _fix_listing("C:\\dir", "somedir/dir/../../", "home")
'C:\\dir\\home'
>>> _fix_listing("C:\\dir", "somedir/dir/../../home/")
'C:\\dir\\home'
>>> _fix_listing("C:\\dir", "somedir/dir/../../home")
'C:\\dir\\home'

[wardi]

I just pushed a fix like yours. Would you care to help me find other places that have the same problem? (uploads, directory creation, file version/thumbnail creation)

I tried to check the upstream code to see if the issue was fixed there, but the code has changed dramatically so it's not obvious to me.

[pahaz]

I am add some validations.

ROOT_DIR = os.path.abspath(os.path.join(fb_settings.MEDIA_ROOT, fb_settings.DIRECTORY))
# chroot directory for all file namitulation, and We must not allow to get access out of ROOT_DIR dirs.

I am add two function.

_check_access - Function for check access to path, and if access allow then return absolute path to file, else then raise exception (similar as we raise exception if path is None).

_check_name - this function for check correct naming dirs or files. Checking similar check in form (but in form check we can see problem with names as '../../name').

Maybe I misunderstood the logic of FOLDER_REGEX better not ispolzvat it.

This fix is needed for people who use django admin app for all users (no stuff). I have seen many such examples of the use django admin app.

I will try to reconsider the logic of correction, maybe I'm mistaken, during the week I will look again (sorry I did it in haste.)

And similar fix needed for admin app with grappelli.

I apologize for my English.

[pahaz]

If you really want to improve this app, you have to take any steps to resolve the issue.
Because the vulnerability can at least look at the code project.
And as the maximum gain full access to the operation system.
I think you understand the danger of this problem.

[wardi]

Yes, this is a serious issue. I'll look at your other changes when I have a few minutes.

[wardi]

Your changes look pretty good. Creating a module-level ROOT_DIR does interfere with some dynamic sites monkeypatching I'm doing, so I'll make a small change there and do some testing.

[pahaz]

I am find real security error, when any user can upload any file in any dirs in os with the rights of web applications.
Strange, veary strange.