FriendsOfFlarum / upload

The file upload extension with insane intelligence for your Flarum forum.
https://discuss.flarum.org/d/4154
MIT License
177 stars 96 forks source link

Add client file extension whitelist feature #225

Closed clarkwinkelmann closed 4 years ago

clarkwinkelmann commented 4 years ago

Fixes #224 Fixes #221 Related #9

I have pondered a few solutions for this issue and I don't think we can make a generic solution that works out of the box for everyone.

So this solution introduces a whitelist for files we won't attempt to rename.

I believe it also fixes a security issue where you could get the original client extension be used for files for which we couldn't guess the extension, which kind of defeats the whole purpose.

Now the extension defaults to bin if no other extension can be determined.

The client-provided file extension is only used if it's in the whitelist.

This solution should even cover the case of those who wanted to whitelist PHP files (we've had this request in the past). It's the responsibility of the forum owner to check the file extensions they whitelist won't cause issues with the adapter.

Question for reviewers: should I add the ability to whitelist everything? For example by inputting * in the field. This would be a very bad idea with a local adapter, but for other adapters it could make sense.

eddiewebb commented 4 years ago

So this solution introduces a whitelist for files we won't attempt to rename.

Perfect!

should I add the ability to whitelist everything? For example by inputting * in the field.

I think you can do without wildcard for now, this applies mostly to "specialty" cases where tools re-package zip or xml, and the number of extensions low.

Just my 2 cents, thanks for great software.