Secure Image upload

[transparent]

I'm building a website that will allow users to upload images.

I want to ensure that they only upload jpg or gif.

I could just check the extension, but that seems very insecure to me.

So, I want to prevent people being able to upload files (an exe for example) with a jpg or gif extension.

Further, I want to flag images that may be inappropriate i.e. I want to prevent people uploading porn. I was thinking I could look at the pink content?

Thoughts?

 

[ca8msm]

I was thinking I could look at the pink content?

Is that a serious suggestion?!


____________________________________________________________

Need help finding an answer?

Try the Search Facility or read FAQ222-2244 on how to get better results.

 

[HarleyQuinn]

You could check the blue content of the image

HarleyQuinn
---------------------------------
Help us to help you,
read FAQ222-2244 before posting.

 

[transparent]

Rumbled.

Joking aside, what are your thoughts regarding:

I'm building a website that will allow users to upload images.

I want to ensure that they only upload jpg or gif.

I could just check the extension, but that seems very insecure to me.

So, I want to prevent people being able to upload files (an exe for example) with a jpg or gif extension.[/qoute]

 

[ca8msm]

In all honesty I don't think you will have a problem (if you only allow certain extensions) as even if the user uploads an exe file (that they have changed the extension to jpg) then the file will be called myfile.jpg on the web server and therefore cannot be executed like an executable file.

Unless they somehow have access to change the extension of a file that has been uploaded to the server (then that would be insecure) you should be fine.


____________________________________________________________

Need help finding an answer?

Try the Search Facility or read FAQ222-2244 on how to get better results.

 

[sirjon]

Hi,

Try using these functions, I found them on the internet and they work for me.

Public Function IsImage() As Boolean
Dim context1 As HttpContext = HttpContext.Current
If Not Me.IsMultiple Then
Dim file1 As HttpPostedFile = context1.Request.Files.Item(0)
Return Me.IsImage(file1)
End If
Return False
End Function

Protected Function IsImage(ByRef File As HttpPostedFile) As Boolean
If File.ContentType.StartsWith("image") Then
Return True
End If
Return False
End Function

HTH

j

----------------------------------------------------------------------------------------
A community development project in asp.net -

http://www.whatsthehampton.com/

 

[transparent]

Nice idea but the problem is that MIME types can easily edited, so you could actually upload a file of any type still.

Cheers for your thoughts

 

[transparent]

I could read the header of the file.

http://www.obrador.com/essentialjpeg/headerinfo.htm

I'd have to write a class the excepted a byte array, and using these rules it could parse the header.

Surely somebody will have done this before!!

 

[ca8msm]

As I said before the file would still be called myfile.jpg. What is it that you think can be done with this file when it has this name?


____________________________________________________________

Need help finding an answer?

Try the Search Facility or read FAQ222-2244 on how to get better results.

 

[transparent]

Well, a user could share mp3s with their friends by uploading

isreallymusic.jpg and then passing the URL around.

etc etc

OR

if my code expects and image or jpg and trys to perform GDI operations, my app will crash

etc etc

 

[transparent]

I could try and load the byte stream into an image object, within a try catch, however this is quite resource intensive, so I'd prefer to find a more efficient and elegant solution - such as file header parsing.

 

[ca8msm]

Well, for the examples you have give, I would say solutions to them would be:

isreallymusic.jpg and then passing the URL around.

Simply set a limit to the file size (as presumably you wouldn't want users uploading images that were over a fairly small size anyway, 500k-1Mb for example).

if my code expects and image or jpg and trys to perform GDI operations, my app will crash

Error handling should always be used and therefore your application wouldn't crash - it would inform the user that the file is not in the correct format.

I'm sure there are ways to read each file and get the actual type (but there are probably also hacks that people can use to mask this). For the things that you seem to be worried about, I think going down that route may be overkill.



____________________________________________________________

Need help finding an answer?

Try the Search Facility or read FAQ222-2244 on how to get better results.

 

[chrissie1]

I would look for black triangles.

Christiaan Baes
Belgium

If you want to get an answer read this FAQ faq796-2540
There's no such thing as a winnable war - Sting

 

[transparent]

Using try/catch statements all over the place results in a sluggish application when under heavy load.

I agree limiting file size is important, if only to prevent people uploading huge images!

 

[ca8msm]

Using try/catch statements all over the place results in a sluggish application when under heavy load.

Error handling really doesn't have much speed implication at all and especially not when they are used correctly. For example, if your calling function has a try/catch then the actual function doing the work can error and the calling function will handle the error.


____________________________________________________________

Need help finding an answer?

Try the Search Facility or read FAQ222-2244 on how to get better results.

 

[SqueakinSweep]

I would look for black triangles.

Good idea Chrissie...but you havent taken natural blondes into consideration.

Sweep
...if it works dont f*** with it

 

[transparent]

try blocks themselves arent processor heavy, BUT throwing exceptions is.

As is catching exceptions.

try
{
}
catch(Exception e)
{

}

 

[transparent]

In fact Microsoft say:

Exceptions add significant overhead to your application. Do not use exceptions to control logic flow, and design your code to avoid exceptions where possible. For example, validate user input, and check for known conditions that can cause exceptions. Also, design your code to fail early to avoid unnecessary processing.

i.e. the exception introduced in the method suggested (by myself) will probably put more load on the server than if I validate the data myself.

 

[ca8msm]

What I think microsoft are saying in the above quote, is that you should do as much validation as you can to avoid errors (and not use it to control logic flow) but errors can still occur and try/catch statements should still be used in case any other errors happen. This is a far better way than what you suggested in which your application would simply crash and the error would be handled by the worker process.

You should also have error handling in if you are going to be using GDI+ to perform certain functions as just because an image has a jpg extension and has jpg headers, it doesn't mean that the file is valid and can be opened correctly.


____________________________________________________________

Need help finding an answer?

Try the Search Facility or read FAQ222-2244 on how to get better results.

 

[transparent]

I didn't say that the application should crash.

I said that you shouldn't use try/catch for logic flow i.e. validation of user input.

For example:

try
{
int i=int.Parse(s);
}
catch (Exception ex)
{
// is not anumbe
}

Is very bad practice.

As is:

try
{
//load image object from uploaded file
}
catch (Exception ex)
{
// is not an image
}

What should be done is

if(IsJpeg(file))
{
//do something
}

Where IsJpeg avoids exceptions.