Basic image validation for logo/favicon

[havardox]

Currently, a person can upload any file as a favicon or logo under theme settings. This could be a security issue if a malicious user gains access to the theme settings.

Suggestion: there should be some basic header validation to prevent users from uploading non-image files. Also should limit the size of the upload (i.e. under 3 MB).

[merwok]

Would migrating the favicon field from FileField to ImageField do the job here?

[havardox]

I think that would work. Although the file size would need to be manually checked.

[merwok]

I’m not sure about that after all. Maybe using proper size for images is a documentation / teaching issue between developers and users of an admin panel, not a technical constraint.

This could be a security issue if a malicious user gains access to the theme settings.

If a malicious user is in your admin, you have bigger problems!

[havardox]

Still, it would be nice if there were some basic sanity checks. E.g. if it's a internal tool and some idiot decides to upload a 2GB+ file. Stuff like that.

[fabiocaccamo]

@havardox

• the favicon field is a FileField to support also .ico files
• to avoid the upload of 2GB+ file it is possible to use the FILE_UPLOAD_MAX_MEMORY_SIZE django setting
• as @merwok said, if a malicious user is in your admin, probably you will have bigger problems

I think that the best thing that could be done here is to add 2 validators to the fields, one for check file size limit and another one for check file extension.

[merwok]

Ah, I didn’t think that it would be important to support the old ICO type for a modern website.

check file extension

If you rely on extension, please validate that the actual file contents match the type guessed from extension.

[fabiocaccamo]

@merwok when I started to abstract this library in my existing projects I used .ico files, actually the FileField could be replaced by an ImageField.

[merwok]

Turns out that django checks file extension only*, and relies on Pillow to determine all extensions supported by the system. ICO is marked as always supported in Pillow docs.

(* but it also uses PIL.ImageFile.Parser to get the image’s dimensions, so maybe that step rejects fake images, or maybe not because RuntimeError is caught there. needs to be tested!)

[fabiocaccamo]

It was long time ago (maybe 8 years), but I remember that the .ico with ImageField raised an Exception, so I used a FileField.

[fabiocaccamo]

You can upgrade to 0.16.2 version.