search

mysociety / popit-api

DEPRECATED - Development on PopIt has stopped and it is no longer being maintained
https://goo.gl/Vvej4Q
Other
17 stars 3 forks source link

Disallow non-image uploads & ignore the supplied mime_type #139

Closed mhl closed 9 years ago

mhl commented 9 years ago

It seems unnecessary to trust the user of the API to get the MIME type of an uploaded image correct, when we can infer the file type using libmagic. (And this saves documenting the mime_type parameter...)

Since libmagic just looks for invariant magic numbers at fixed offsets into binary files (to paraphrase the man page), it can't tell whether a file is malformed (possibly maliciously), but this was a risk with the existing code anyway, and if a browser or image library is subject to buffer overruns or the like that's a much more serious problem than just that PopIt might store and serve such a file.

In addition this makes sure that newly uploaded files will only be served with an image/* Content-Type. (Existing uploaded images will still be served with whatever mime_type was specified on upload.)

Fixes mysociety/popit#810

mhl commented 9 years ago

Actually, I'm suspicious of this having just tried an image upload locally to a person who had existing images that were hosted on the live site, and finding that afterwards the images element in my local PopIt instance was empty, rather than having the existing two images + the new one. I'll have to investigate that more...

mhl commented 9 years ago

I can't reproduce that problem now :/

chrismytton commented 9 years ago

Looks good to me! :rocket: