9

after reading the data from the request as Stream i needed to convert it to Image so i used this Method:

Stream inputStream = HttpContext.Current.Request.InputStream;
Image img = Image.FromStream(inputStream)

so while im unable to know what was in that inputStream (file format is done already but still..) there may be a virus or malware, so in that case FromStream(Stream) would throw ArgumentException. since it's not an image.

my question is - If the file uploaded contains a virus, and the method throws an exception while trying to convert the stream to Image: did that make any harm to the server? if so, how to avoid it? what should be the scenario for handling file upload in the server? im just wondering rather or not i need to scan for viruses in the scenario of photo-uploading.

AviD
  • 72,138
  • 22
  • 136
  • 218
Yaniv
  • 191
  • 1
  • 2
  • It depends if you trust the method not to have any sort of buffer overflow bugs within it. I suggest not loading the stream blind. – Ramhound Feb 21 '12 at 18:26
  • Hi Yaniv, welcome to [security.se]! As @makerofthings7 noted below, you can find more general guidance regarding securing file uploads at [How can I be protected from pictures vulnerabilities?](http://security.stackexchange.com/q/8587/33). However your question does add the specific focus on this specific API, though the short answer is "there is no benefit here, see the other question" :) – AviD Feb 21 '12 at 20:53

2 Answers2

7

If the uploaded file is in a valid image format, .FromStream will not throw an exception, even if it contains a virus. That is, there is no virus checking in it.

On the other hand, there is also no way for the virus to activate, since the file at this point is only being handled as raw bytes.
(It is certainly possible, if not feasible, that it contains an attack targeted directly at the .NET Image class, e.g. via buffer overflow. However, besides being highly improbable that such a vulnerability exists, it's even more unlikely that someone will go to the trouble of targeting such a niche vector).

So, at this point you are perfectly (realistically speaking) safe, but neither do you have any idea if the file is clean or not.

The real question becomes, what happens with this file afterwards?

Is it saved to disk, and used locally?
In this case you should have some form of anti-virus check it, if only for your benefit - if you believe in them, you can just have the local AV engine scan it.

Is it later returned to other users, and thus can use your system for spreading?
You should definitely have some form of AV scan before saving the file. Note that this may not be as simple, since you might be storing the images in your database (there are AV scanners that can be activated by API for a memory segment, before it gets saved to file)... In any event, I am categorically opposed to running antivirus scans directly on your actual systems. You'd be better off having some form of gateway AV scanner, before it even hits your system.


However, take into account that when you allow random users to upload arbitrary files, virus cleaning is not the utmost of your worries.

  • For one, you can be swamped - either by huge files, or many smaller ones, thus DoSing your server.
  • If the user can specify the saved file path - he could potentially overwrite system files, or upload executable code.
  • If file attributes are saved, and later displayed (this includes file name and username, but also description, file type, location, etc) - this can lead to other standard web attacks, such as XSS or SQL Injection.
  • Do a search here for GIFAR - these are files that are both valid image files, and valid JAR (Java Archive - executable code for the browser) files. These are of course not a virus...
  • Depending on the system, you may be at risk for legal damages, if your users upload e.g. copyrighted images, or e.g. child pornography. Of course, consult with your lawyer, as I am not one.

TL;DR

In short, accepting arbitrary uploads does lead you into a whole mess of potential trouble, and virus scanning is not the worst of it.

AviD
  • 72,138
  • 22
  • 136
  • 218
4

Yes, the function is protected from all known buffer overflows, as Microsoft sends out patches for the .NET codebase. But it's not a complete solution.

For more information see these links:

Here are the protections I would employ when accepting images from untrusted sources:

  • Patch the server: The Image class is part of the .NET framework so as long as you're on a supported framework, MSFT will prevent buffer overflows.

  • Handle the ArgumentException from this class, since attempts to upload non-images will throw an exception. You can get early warning of attempted "hacks" with this exception.

  • Check the extension and name of the image

  • Don't allow the user to specify the directory (or control it tightly if you do)

  • Check the content type against the extension.

  • Run a virus checker against the image (e.g. VirusTotal)

  • Use the header X-Content-Type-Options : Nosniff (more info) to protect from GIFR-style attacks.

  • Don't show image metadata to the end users. Tools like Exiv2 allow the viewing and editing of untrusted data.

  • Size Limit: Wrap the Stream class and allow that to cut off attempts to upload large files. If you're using ASP.NET then by default the server side component saves all the data to RAM first. Avoid memory exhaustion DoS by using a HTTPHandler that saves directly to disk.

  • Use a separate DNS domain for serving images (more info)

  • Convert all uploaded images to BMP, then to a common format (PNG, etc) (more info)

  • Ensure that the above-mentioned conversion staging area is inaccessible to the public

  • Inject the picture with noise (more info)

Glorfindel
  • 2,235
  • 6
  • 18
  • 30
makerofthings7
  • 50,090
  • 54
  • 250
  • 536
  • so you say that ArgumentException can save me from viruses uploaded by image files? – Yaniv Feb 21 '12 at 18:33
  • @Yaniv - By itself...That would be foolish. You should do all of the things makerofthings7 suggested. The function has no known buffer overflow bugs within it. That of course doesn't mean they don't exist. **Everyone thought WPS was secure also...** – Ramhound Feb 21 '12 at 19:20
  • +1 for the practical recommendations, though I'm wondering now if this question is not a dupe of the other sourced one... – AviD Feb 21 '12 at 19:44
  • @AviD - Yes, I started researching the question and saw how similar this is to the other questions. The only think that prevented me from flagging this as a duplicate is that this Q is was scoped to a particular .NET API, and not broad like the others. Also Yaniv is new to Sec.SE and may have not expected quality answers elsewhere on the site. – makerofthings7 Feb 21 '12 at 20:03
  • Thanks AviD for your kindness and the full answer you wrote me above. im new to Sec.SE, this is my very first post here. i'll read it all, vote and everything as needed. uploading files is a headache, but i must do it. – Yaniv Feb 22 '12 at 04:39
  • wow makerofthings7, that's sounds like a lot of work. but i'll do it. thanks! – Yaniv Feb 22 '12 at 04:52
  • @Yaniv - See this question for additional things to think about: http://security.stackexchange.com/q/11995/396 – makerofthings7 Feb 23 '12 at 15:08