prevent XSS in renaming dirs/files

[samialfattani]

Fixes #1506

This PR to prevent XSS security issue on renaming dirs or files

[ElLorans]

LGTM, will wait a couple of days before merging in case somebody wants to chime in.

[samuelhwilliams]

I don't think this does any damage, but I don't think it's fair to say it fixes #1506 really. If there's any bad data already in someone's system then this won't help them.

We should make sure that we treat filenames as dangerous input everywhere they're rendered, and make sure they're escaped appropriately.

If #1506 is talking about this code, then we should run it through Jinja2's escape filter or similar.

For some reason I thought that would be on by default but maybe it's not?

I think happy to merge this, but not to mark 1506 as fixed.

[ElLorans]

Thanks for the input. @samialfattani , can you move to using something like werkzeug.secure_filename ? Hopefully you can replace a part of your code.

[ElLorans]

Sorry for not being clear, but I was thinking of replacing the regex with a check between the input and the result of secure_filename. Replacing the validator with secure_filename like you are doing represents a breaking change since you are silently renaming the file. Do we think this is a problem?

[samialfattani]

Sorry for not being clear, but I was thinking of replacing the regex with a check between the input and the result of secure_filename. Replacing the validator with secure_filename like you are doing represents a breaking change since you are silently renaming the file. Do we thing this is a problem?

oh !! i spend a lot of time to replace the regex i hope this will not go for nothing. anyway i don't think that renaming the filename or directory is a breaking change since this technique is actually used in /rename and other places in the project. that's why i thought it is better to use the same way.

[samialfattani]

any thing i can do from my side ?