← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands-website/pybb_attachments into lp:widelands-website


I am a bit lost here...

Some checks are made now, but i am unsure if we need them all. What is implemented:

1. Link of Attachment:
If a user clicks on a link of an attachment, he will be ever prompted for a download (or to open the file with an associated program), even if it is a file which the browser may can display, e.g. an image (png, jpg) or text file. This is not optimal, but i think it is more secure.

2. Restricting upload permission
A user has only permission to upload files, if he has had written 5 posts prior. The number of 5 derives from MAX_HIDDEN_POST at the moment, but i think it's better to have an own value for this. What number do we want?

3. Allowed extensions:
Added a list:
ALLOWED_EXTENSIONS = ['wgf', 'jpg', 'jpeg', 'gif', 'png', 'ogg', 'lua', 'ods', 'zip', 'json', 'txt', 'csv']
I am unsure if this list will be ever complete. E.g. some may want to upload some other types of files (e.g. py, odg, doc, ...)

4. Extension handling:
- Files without extension aren't allowed (can be problematic e.g. with the campvis file)
- For 'wgf' files: Check if some base entries can be found (e.g. '/binary/' or '/minimap.png')
- For image files: Check if this is really an image using the Pillow library. This finds corrupted .png but not corrupted .jpg
Do we need a check for each allowed extension?

5. Comparing Mime-Types:
When uploading a file, a Mime-Type (content-type) is also submitted. The submitted mime-type is mostly derived from the extension of the file. E.g. if you rename a file from 'image.png' to 'image.txt' the OS (or browser) sends the Mime-type 'text/plain', which is obviously wrong. To prevent such mime-type mismatch, a check is made which compares the sent mime-type with the mime-type derived from python-magic. e.g. python magic will show for 'image.txt' the mime-type 'image/png'.

As said, i am a bit lost, not really knowing if we need all this checks, or if they are enough at all :-S Any opinions?
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands-website/pybb_attachments into lp:widelands-website.