Skip to content
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

Security problems with move / rename / etc. #5

Open
alanswx opened this issue Jan 16, 2018 · 7 comments
Open

Security problems with move / rename / etc. #5

alanswx opened this issue Jan 16, 2018 · 7 comments

Comments

@alanswx
Copy link
Collaborator

alanswx commented Jan 16, 2018

If I do a move with the dialog, and put in a path of ../../../hello it seems to move the file / directory beyond the root of our safe spot. I started working on a patch, but I think it effects copy/delete/edit etc. It seems like the upload safe's the name - I am not sure if I can hack the form and have it upload anywhere.

@alanswx
Copy link
Collaborator Author

alanswx commented Jan 16, 2018

I think this is fixed in #6

@alanswx
Copy link
Collaborator Author

alanswx commented Jan 19, 2018

I think the ajax request for download doesn't have a path check.

@jsooter
Copy link
Owner

jsooter commented Jan 19, 2018

Thanks for doing that @alanswx

@alanswx
Copy link
Collaborator Author

alanswx commented Jan 22, 2018

Also - in move, I didn't check the source, only the destination. You could move "/etc/passwd" into the volume probably..

@jsooter
Copy link
Owner

jsooter commented Jan 22, 2018

  1. Don't run this as a privileged user and that will solve most file system security issues
  2. What I didn't think of is that it could modify it's own source and that should probably be prevented. A simple function to check the path should work. It could be done before the request gets routed by Flask.

@alanswx
Copy link
Collaborator Author

alanswx commented Jan 22, 2018

I think we could put in a few more checks. I am confused about all the code paths. If we could test each of them with a bogus path that would be great. I wonder if some of this code isn't needed - since they changed the API around.

@jsooter
Copy link
Owner

jsooter commented Jan 22, 2018

When I wrote it I used the wiki as spec. It really needs to be reviewed again since the update.
https://github.com/servocoder/RichFilemanager/wiki/API#methods

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants