2

For a webapp based around user uploaded pics, I want to check that I'm doing things sufficiently securely. Below is an outline of the current setup.

  1. Of course only registered users can upload, but I know that doesn't mean anything.
  2. The site is exposed via nginx.
  3. The machine on which the webserver is hosted only accepts incoming connections from the nginx machine.
  4. The image upload link (nginx directive) is rate limited (in a way that generously accounts for a legitimate user spike).
  5. The webserver accepts only jpg/png/tiff/webp image formats (it derives the mime type using the magic library).
  6. Both nginx and the webserver have a limit on the accepted file size.
  7. All uploaded files are saved using a filename which is a hash of the contents.
  8. By default, they are saved with only rw permissions, not rwx.
  9. The images are stored on the same system as the webserver.
  10. They are saved directly to the final location (there's no intermediate staging location) but the images are not available for viewing until a specific database column is updated (see next point).
  11. A periodic virus scan is completed on the newly uploaded files (and any malicious files are removed to a different directory to which the servers have no access) and then the aforementioned database column is updated to make the files available for viewing.
  12. Lastly, a tiny amount of random noise is added to each image by doing something like magick image.jpg -evaluate Add 0.1 image.jpg

What vectors am I missing? The goal is not to protect against a state actor, but to have reasonable protection against the average (or even somewhat skilled) malicious irritant.

  • 1
    Validation phase: ensure that uploaded file extension always matches the detected format, and the file can be properly decoded as such. You can also consider stripping various metadata, like EXIF tags - basically, add a processing step, that takes only what's necessary (only image data) and save as a new file, with this step running as much isolated as possible (so RCEs in image decoding libraries won't get attacker any much access). After this, ensure that nginx always responds with the correct Content-Type and all the safety headers, X-Content-Type-Options etc. And I think it should be good. – drdaeman Jun 10 '18 at 19:33
  • 1
    Why do the virus scan periodically instead of on every new file? Sounds way, way saner. – Marcus Müller Jun 10 '18 at 19:39
  • 1
    Also, imagemagick is a very well-maintained library, but it's also a very large attack surface, and there's been a lot of critical breaches happening through it. Using a much more restricted library for validation might be a good choice. – Marcus Müller Jun 10 '18 at 19:41
  • @drdaeman Thank you for the suggestions. I'll build them in. Follow up question about the validation part - ensuring that extension type matches the mime type, could it be an actual attack vector? As in, could there be a security consequence to calling a png file image.jpeg? – Yogesch Jun 10 '18 at 19:42
  • 1
    @Yogesch file formats like .png and .jpeg allow for Polyglots. I.e. a valid JPEG can also be a valid PDF and a valid x86 boot sector. This at least limits the potential type confusion later. – Marcus Müller Jun 10 '18 at 19:43
4

I think the only way to answer this is through a collaborative commenting effort, so here goes a community wiki:

  1. Of course only registered users can upload, but I know that doesn't mean anything.

Agreed.

  1. The site is exposed via nginx.

Which means you have the actual web application running behind that – make sure that your adapter (WSGI? fCGI?) is securely set up.

  1. The machine on which the webserver is hosted only accepts incoming connections from the nginx machine.

clever move!

  1. The image upload link (nginx directive) is rate limited (in a way that generously accounts for a legitimate user spike).

Make sure this doesn't make it easier to DoS-attack your site.

  1. The webserver accepts only jpg/png/tiff/webp image formats (it derives the mime type using the magic library).

magemagick is a very well-maintained library, but it's also a very large attack surface, and there's been a lot of critical breaches happening through it. Using a much more restricted library for validation might be a good choice.

ensure that uploaded file extension always matches the detected format, and the file can be properly decoded as such. You can also consider stripping various metadata, like EXIF tags - basically, add a processing step, that takes only what's necessary (only image data) and save as a new file, with this step running as much isolated as possible (so RCEs in image decoding libraries won't get attacker any much access). After this, ensure that nginx always responds with the correct Content-Type and all the safety headers, X-Content-Type-Options etc.

  1. Both nginx and the webserver have a limit on the accepted file size.

good.

  1. All uploaded files are saved using a filename which is a hash of the contents.

Make sure to salt that hash; hashes are crackable (within limits), and you don't want an attacker to control the file name of their uploads. Don't just use MD5, but a less broken hash.

  1. By default, they are saved with only rw permissions, not rwx.

Save them with read permissions. Once they are saved, they never need to be modified again.

Also, use SELinux (or your server OS'es equivalent) to make sure your web application can only write and read that directory, never execute files in it.

  1. The images are stored on the same system as the webserver.

Might very quickly become a scalability issue: Server software would these days preferably run on fast reliable storage media (SSDs with hot fallover), whereas your user pictures would sufficiently be stored on cheap storage media with slow backups (i.e. if two hard drives fail at the same moment, OK, you'll need 15 minutes to restore from your redundancy and might lose a couple minutes of new user images).

  1. They are saved directly to the final location (there's no intermediate staging location) but the images are not available for viewing until a specific database column is updated (see next point).

  2. A periodic virus scan is completed on the newly uploaded files (and any malicious files are removed to a different directory to which the servers have no access) and then the aforementioned database column is updated to make the files available for viewing.

There's no computational advantage to doing that periodically instead of instantaneously after uploading the file. So, do it instantaneously, and keep your storage load constant as possible. Running the antivirus as a daemon avoids having to load up the virus definitions into memory for each scan.

  1. Lastly, a tiny amount of random noise is added to each image by doing something like magick image.jpg -evaluate Add 0.1 image.jpg

That is a very compute intense step, which uncompresses images, adds noise, and that's the worst part, which in case of JPEG leads to maximum quality loss if you want to keep the original file size. (noise is very hard to compress, unless it's so weak that it actually gets compressed away and the resulting image is actually as if you never added noise.)

I don't see any security advantage of this step.

Further points

  1. Containerize your web application.
  2. Use a separate logging server
  3. Monitor for sudden increases in storage consumption – in case someone abuses your site automatedly e.g. to host pictures for their own gain
  • "Make sure to salt that hash" - I believe this is excessive. Just generate a random string (optionally prefixed with a hashid or just a hashid if you don't want complete unpredictability) - that would do the trick just as well, and attacker won't have any control of the filename this way. As for the content hash (SHA256 should be a good choice) - store it in the database. This is not for security's sake but just to avoid duplicate uploads. – drdaeman Jun 10 '18 at 20:09
  • "instead of instantaneously after uploading the file. So, do it instantaneously" - not exactly instantaneously, but queue the processing, and make sure queue consumers can be scaled up horizontally (i.e. if you'll need to speed it up you'll be able to just throw new machines at it). Ensure the queue is adequately monitored and you receive proper alerts if it gets clogged. Be nice to users if they expect their uploads to show up somewhere - then show them (just the owners) a placeholder, with "your image is there, awaiting for processing" - like YouTube does it. I'd say it makes much better UX. – drdaeman Jun 10 '18 at 20:16

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.