Ajax avatar image upload without refreshing page

[mlukac89]

Hi all

 

Can anyone help me with ajax for upload avatar on user profile, i just need to check if all of this if fine to upload image or throw error if any error exists, and without submit button.

<?php

$target_dir = "uploads/";

$uploadOk = 1;

// $check is array with image data
// 0 - width
// 1 - height


// Check if image file is a actual image or fake image
if(isset($_POST["submit"])) {
	$target_file = $target_dir . basename($_FILES["image"]["name"]);
	$imageFileType = pathinfo($target_file,PATHINFO_EXTENSION);
    $check = getimagesize($_FILES["image"]["tmp_name"]);
    if($check !== false) {

		// Check if file already exists
		if (file_exists($target_file)) {
		    $error[] = "Sorry, file already exists.";
		    $uploadOk = 0;
		}
		// Check file size
		if ($_FILES["image"]["size"] > 500000) { // 500 kb
		    $error[] = "Sorry, your file is too large.";
		    $uploadOk = 0;
		}
		// Allow certain file formats
		if($imageFileType != "jpg" && $imageFileType != "png" && $imageFileType != "jpeg" && $imageFileType != "gif" ) {
		    $error[] = "Sorry, only JPG, JPEG, PNG & GIF files are allowed.";
		    $uploadOk = 0;
		}

		// check image width and height 150x150px max size
		if ($check[0] > 150 && $check[1] > 150) {
			$error[] = "Image too large max upload 150x150 px.";
			$uploadOk = 0;
		}

		// Check if $uploadOk is set to 0 by an error
		if ($uploadOk == 0) {
		    $error[] = "Sorry, your file was not uploaded.";
		    
		// if everything is ok, try to upload file
		} else {
			//echo print_r($check);
		    if (move_uploaded_file($_FILES["image"]["tmp_name"], $target_file)) {
		        $error[] = "The file ". basename( $_FILES["image"]["name"]). " has been uploaded.<br />";
		    } else {
		        $error[] = "Sorry, there was an error uploading your file.";
		    }
		}

        $uploadOk = 1;
    } else {
        $error[] = "File is not an image.";
        $uploadOk = 0;
    }
}


?>

<h1>Image upload test</h1>

<?php

if (!empty($error)) {
	echo 'There are folowing errors: <br />';
	foreach ($error as $key) {
		echo '<ul><li>' . $key . '</li></ul>';
	}
}

?>

<form action="" method="post" enctype="multipart/form-data">
    Select image to upload:
    <input type="file" name="image" id="image">
    <input type="submit" value="Upload Image" name="submit">
</form> 

Thanks in advance.

[Jacques1]

This is not fine.

 

While you're at least doing some basic checks, this code is still extremely insecure. To name just a few problems:

• You happily accept multiple extensions as long as the last extension is an image extension. So anybody can upload a file named “malicious.php.jpg” with embedded PHP code, and chances are your webserver will actually execute it.
• You let the user choose an arbitrary filename, and that name is then reserved forever. This is obviously a problem, because common names will be taken after a short while.
• It's even possible to create hidden files by simply uploading a file without a name (e. g. “.jpg”).
• You cannot use file_exists() in a web application where many processes may run in parallel. If two processes request the same unreserved name at (almost) the same time, then file_exists() returns true in both processes. So one of the uploads will later be overwritten by the other.

Implementing a secure and robust file upload is a lot harder than it may seem. I strongly recommend that you read up on common risks before you jump to the implementation. Basic security requires at least the following steps:

1. You must choose the filename while making sure that no two processes will ever get the same name. If the username is already unique and every user can only have single avatar, you might simply choose the username. Another typical solution is to use a random number generator like openssl_random_pseudo_bytes() to create unique names.
2. If possible, store the uploads outside of the document root and make them accessible through a PHP script. This will prevent a lot of security problems. For example, you can explicitly set the right Content-Type without relying on the webserver to figure it out (so you don't even need a file extension). There's also a much smaller risk of malicious server-side code being executed.
3. If you do put the uploaded files directly into the document root, extra care must be taken. You need to choose the file extension, and you must explicitly disable script execution in the upload directory.
4. Use Content Security Policy to prevent execution of client-side scripts. For example, the header Content-Security-Policy: default-src 'none'; sandbox will block any JavaScript code that may be embedded in the file.

[scootstah]

You happily accept multiple extensions as long as the last extension is an image extension. So anybody can upload a file named “malicious.php.jpg” with embedded PHP code, and chances are your webserver will actually execute it.

They're only using the extension to check if it's one of the allowed image types. But, this will prevent non-image uploads:

$check = getimagesize($_FILES["image"]["tmp_name"]);
if($check !== false) {

There's a still the problem that the extension is meaningless for what they're trying to achieve, but at least the uploaded file will be an image.

[Jacques1]

No, it doesn't.

 

Just because a file is formally an image doesn't mean it's free of any malicious code. For example, the PNG format allows arbitrary text comments within an image. So it's perfectly possible to craft a file which behaves like an image, even looks like an image but immediately turns into malware as soon as the webserver executes it.

 

There is no image/non-image duality. A file can be both, depending on the context.

[scootstah]

getimagesize() will return false if the given argument is not an image. It checks the MIME type. Yes, the file could contain malicious code even if it is also a valid image. But such a file is generally still safe to store and use, and the only problem would come from a vulnerability in the viewer.

 

Checking the MIME type is the best that you can do.

[Jacques1]

No, it's not.

 

You're assuming that the webserver will determine the MIME type of a file by scanning the content, but that's generally not the case. Many webservers only check the extension, so a file called “foo.php” will in fact be treated as a PHP script, even if its structure happens to resemble an image.

 

You're also assuming that all MIME detection heuristics are the same, but they're vastly different. There's an entire class of attacks which consists of assembling “chameleon files” to exploit those differences. For example, the server may interpret a particular file as an image, but a UA may interpret the exact same file as an HTML document.

 

So, no, checking the file with getimagesize() doesn't do much. I'm not against it, but it's one of the less important security measures.

[mlukac89]

I was never worked with upload in php, but if u have function for secure output and input how it can run malicious code ?

 

Btw this is script from w3schools, i know its just working examples without security, but cant i check if filename have any extension before .jpg or other format ? and if have to give error without upload ?

 

I think i can acomplish this with strpos() and stip_tags() funtions.

[Jacques1]

No, no, no.

 

First of all, do not copy-and-paste code from w3schools, not even as an example. I feel slighly insulted that you actually wanted us to debug that crap. In fact, do not copy-and-paste any code from any website. 99% of it is absolute garbage and will turn your server into a playground for script kiddies.

 

Secondly, forget about strip_tags(). It's a shame that this function even exists in PHP, because it's complete nonsense. All it does is mangle the input and leave you with broken data.

 

Third, forget about assembling your own extension check with strpos(). Thousands of people have tried it before you, and they've all failed miserably. As I've already said above, you need to choose the extension. Don't allow the user to do it.

Edited October 12, 2015 by Jacques1

[scootstah]

Many webservers only check the extension, so a file called “foo.php” will in fact be treated as a PHP script, even if its structure happens to resemble an image.

Correct, which is why you save it to the filesystem with the correct corresponding extension, and not the one that the client supplied.

 

It sounds like you're saying checking the MIME type is pointless. How else would you determine whether a file is an image or something else?

[Jacques1]

I repeat: You cannot tell if a file “is an image”. What does that even mean? Is a PHP script an image if only it starts with a PNG signature? Is it an image if I put the code into a bunch of PNG chunks?

 

Your personal definition of an image is completely irrelevant. The question is: How does it get interpreted? And that's a lot more complex than doing a bit of PHP MIME detection magic. As I've already said, I'm not against MIME heuristics. They can catch obvious user errors, and they may increase security a tiny bit. But that's really just a bonus feature. All other measures I've mentioned are much, much more important.

 

Either way: Claiming that a file is safe if only it has passed the getimagesize() check is definitely wrong.

 

 

[mlukac89]

What about pure jquery upload ? is it safer ?

 

Btw getimagesize() returns array with data about image ["mime"], why u cant check then ?

 

And 1 more thing can u show me example then how i can make safe upload for jpg, jpeg, gif, png and filesize not more than 50 kb ?

 

array(5) {
      [0]=>
      int(159)
      [1]=>
      int(91)
      [2]=>
      int(13)
      [3]=>
      string(23) "width="159" height="91""
      ["mime"]=>
      string(29) "application/x-shockwave-flash"
    }

[scootstah]

Is a PHP script an image if only it starts with a PNG signature?

Yes, by definition.

 

The question is: How does it get interpreted?

Well, that's the important part. As long as you treat an image like an image, then it doesn't matter if there is embedded code. At that point it falls to the viewer to safely handle the image and lot allow arbitrary code to execute.

 

I'm not against MIME heuristics. They can catch obvious user errors, and they may increase security a tiny bit. But that's really just a bonus feature. All other measures I've mentioned are much, much more important.

So how do you tell the client their file is not of an allowed type? How do you decide which file extension to give the uploaded file? How do you decide which content header to send when you display a file?

[Jacques1]

Yes, by definition.

 

Unfortunately, attackers don't always adhere to our definitions. Maybe we can sue them for violating The Divine Law Of PNG.

 

 

 

As long as you treat an image like an image, then it doesn't matter if there is embedded code. At that point it falls to the viewer to safely handle the image and lot allow arbitrary code to execute.

 

Yeah, let's remove all this silly HTML-escaping from our applications, because if the user executes embedded code, that's definitely their fault. That's not our department.

 

Being safe for viewers is the whole friggin' point of a website. Covering your own ass (meaning: the server) isn't quite enough.

 

 

 

So how do you tell the client their file is not of an allowed type? How do you decide which file extension to give the uploaded file? How do you decide which content header to send when you display a file?

 

It. doesn't. matter.

 

Use the filename, use your beloved MIME heuristics, flip a coin, it doesn't matter.

[scootstah]

Yeah, let's remove all this silly HTML-escaping from our applications, because if the user executes embedded code, that's definitely their fault. That's not our department.

Let me know when my web browser can natively execute malicious PHP code embedded in a file. Considering you haven't brought any alternative to MIME types to the table, what exactly are you saying here? That MIME types are meaningless, file extensions are meaningless, and we simply store whatever file the client gives us? Okay...

 

It. doesn't. matter.

 

Use the filename, use your beloved MIME heuristics, flip a coin, it doesn't matter.

How does it not matter? You just blindly save every file that a user gives you, or what?

[Jacques1]

I'm saying that this is an output problem, not an input problem.

 

Trying to make files “safe” by running them through getimagesize() is like trying to make text input “safe” by applying one of those silly “sanitization” functions. Both approaches are entirely missing the point.

 

The MIME heuristics make even less sense, because they merely check the presence of some magic bytes. That doesn't prevent anything. You might as well force the user to click a checkbox saying “I hereby promise that my file isn't malicious.” How does that help anybody? It's just a useless bureaucratic rule.

 

So the input part is more or less irrelevant. I usually determine the file type by the original extension. Since most people still use Windows which is very picky about extensions, that works reasonably well. If the extension is missing, I might use MIME sniffing as a fallback.

 

What's important is the output part: Isolating the files on a separate (sub-)domain, using Content Security Policy etc.

 

 

 

Let me know when my web browser can natively execute malicious PHP code embedded in a file.

 

I'm talking about client-side code (JavaScript, JScript, whatever).

[scootstah]

A lot more can be affected besides just Javascript in a web browser. Clients come in many forms.

 

So again, the problem lies with the viewer. Since you can't be sure that a file doesn't have embedded code in it, you need to be sure that it can't be executed even if it did exist. I've tested this in the past by embedding PHP into a JPG. The file had an "image/jpeg" MIME type and could be viewed with any image viewer just like a normal JPG. But change the extension to .php and now it is a PHP file. However nothing can happen unless you treat the file like a PHP file. If you include() it, eval() it, or otherwise cause it to be parsed as PHP you'll have a problem. But if you treat it like an image, there is no problem. It doesn't matter if there is PHP embedded unless the application viewing it has a vulnerability that causes the PHP to parsed and executed.

 

I may be wrong but I don't believe simply embedding Javascript into a valid image file is going to cause it to be executed. At least, I can't find anything that says that is the case.

 

EDIT: Also you seem to be under the impression that your suggestions and checking MIME types are mutually exclusive. One should be implementing XSS protections regardless of whether they're handling file uploads or not - that's a given. Checking the MIME type is still important though, unless you want to be storing a bunch of .exe's and .pdf's.

Edited October 13, 2015 by scootstah

[Jacques1]

 

I may be wrong but I don't believe simply embedding Javascript into a valid image file is going to cause it to be executed. At least, I can't find anything that says that is the case.

 

I already pointed you to an article (see #6) which explains the problem of chamaleon files in great detail and describes several concrete attacks. If you search for “mime sniffing xss”, you'll find plenty of other cases. I can also provide an example file for older IE browsers, if that helps.

 

What you really need to understand is that MIME sniffing is no exact science. Just because getimagesize() thinks that a particular file is an image doesn't mean that all UAs will come to the same conclusion. For example, UAs will also scan for HTML tags, so if they find one, they may very well conclude that your “image” is actually an HTML document. In case you don't provide any additional client-side protection (separate domains, CSP), your user is now vulnerable to XSS attacks.

 

Older versions of Internet Explorer will even override explicit content type declarations provided by the server.

 

So server-side MIME sniffing is, at best, a vague approximation of how the file may be interpreted. It's much more important to explicitly declare the content type, isolate the files on a separate domain and set up CSP.

 

 

EDIT: Also you seem to be under the impression that your suggestions and checking MIME types are mutually exclusive.

 

No, I'm not under that impression.

As I already said multiple times, MIME sniffing is OK as long as we all agree that it's just a nice bonus feature to detect errors and maybe increase security a tiny bit. But don't claim that getimagesize() will make the file safe for storage, and don't blame the UA when that turns out to be wrong.

 

 

One should be implementing XSS protections regardless of whether they're handling file uploads or not - that's a given.

 

And one also needs to implement XSS protection specifically for uploads.

 

 

Checking the MIME type is still important though, unless you want to be storing a bunch of .exe's and .pdf's.

 

But it's OK to store as bunch of executables and PDF documents as long as they start with a PNG signature?

I mean, I understand where you're coming from. It “feels good” to at least do something with the data. But if you think about it, it's pretty useless. And storing unfiltered data is what we do most of the time. There's no getimagesize() for text input.

[scootstah]

But don't claim that getimagesize() will make the file safe for storage

I was not intending to make that claim, so apologies if I did. The MIME check is only relevant for the process of saving the file to the server - once it's there you have to take other things into consideration. No, a MIME check cannot prevent chameleons, but it can prevent regular files that you don't want. So instead of being able to upload "evil.php" and then executing it from your browser, you'd have to embed code into the image and figure out a way to make it execute, which is a much more difficult task.

 

It's much more important to explicitly declare the content type,

And how would you know what content type to declare if you don't look at the MIME?

 

And storing unfiltered data is what we do most of the time. There's no getimagesize() for text input.

That's different. Nobody is looking at raw database data directly after saving it like you would with an uploaded avatar image or such.

Edited October 13, 2015 by scootstah

[Jacques1]

So instead of being able to upload "evil.php" and then executing it from your browser, you'd have to embed code into the image and figure out a way to make it execute, which is a much more difficult task.

 

You keep mixing up different security measures. Whether or not you allow a file named “evil.php” has nothing to do with whether or not you perform MIME sniffing.

 

Let's do a real comparison:

 

Option 1: Permitting arbitrary file extensions and arbitrary file content

An attacker can upload a file named “evil.php” and execute embedded code.

 

Option 2: Permitting arbitrary file extensions and performing server-side MIME sniffing

An attacker can upload a file named “evil.php” and execute embedded code.

 

Option 3: Enforcing an image file extension and permitting arbitrary file content (my approach)

An attacker can only upload files like “evil.png” and not execute any embedded code.

 

Option 4: Enforcing an image file extension and performing server-side MIME sniffing (your approach)

An attacker can only upload files like “evil.png” and not execute any embedded code.

 

As you can see: The MIME sniffing has absolutely no effect. It's about the other factors (namely the file extension).

 

And, no, embedding code in an image structure is not difficult at all.

 

 

 

And how would you know what content type to declare if you don't look at the MIME?

 

I already answered that question: I look at the user-provided file extension.

 

Security-wise, both the file extension and the file structure can easily be faked, so one is just as good as the other. And with regard to legitimate users, the file extension tends to be extremely reliable, because a) most people will never mess with it, and b) if they do mess with it, they'll quickly run into problems (at least on Windows which maps extensions to programs).

 

 

 

That's different. Nobody is looking at raw database data directly after saving it like you would with an uploaded avatar image or such.

 

Then store the image as a BLOB in the database, if that makes you feel better.

[mlukac89]

What about

 

Check MIME

Check for file extension

Check file as string like ( fullimagename.ext ) if extension dont match dont upload

Rename image and give random name or number with extension u get from allowed extension array ( like original version replace with random code.jpg )

Edited October 13, 2015 by mlukac89

[Jacques1]

I'd simply check the extension, and if it's valid, generate a random name and append the validated extension:

<?php

function html_escape($raw_input, $encoding)
{
    return htmlspecialchars($raw_input, ENT_QUOTES | ENT_HTML5 | ENT_SUBSTITUTE, $encoding);
}

// the set of all valid file extensions
const VALID_EXTENSIONS = [
    'jpg'  => true,
    'jpeg' => true,
    'png'  => true,
    'gif'  => true,
];



// test data
$user_filename = 'foo.jg';



$user_extension = pathinfo($user_filename , PATHINFO_EXTENSION);
$normalized_user_extension = strtolower($user_extension);

if (array_key_exists($normalized_user_extension, VALID_EXTENSIONS))
{
   $rand_bytes = openssl_random_pseudo_bytes(16);
   $real_filename = bin2hex($rand_bytes).'.'.$normalized_user_extension;

   echo html_escape('The file will be stored as '.$real_filename, 'UTF-8');
}
else
{
   echo html_escape('Invalid file extension. Please use one of the following: '.implode(', ', array_keys(VALID_EXTENSIONS)).'.', 'UTF-8');
}

[scootstah]

Let's do a real comparison:

 

Option 1: Permitting arbitrary file extensions and arbitrary file content

An attacker can upload a file named “evil.php” and execute embedded code.

 

Option 2: Permitting arbitrary file extensions and performing server-side MIME sniffing

An attacker can upload a file named “evil.php” and execute embedded code.

 

Option 3: Enforcing an image file extension and permitting arbitrary file content (my approach)

An attacker can only upload files like “evil.png” and not execute any embedded code.

 

Option 4: Enforcing an image file extension and performing server-side MIME sniffing (your approach)

An attacker can only upload files like “evil.png” and not execute any embedded code.

 

As you can see: The MIME sniffing has absolutely no effect. It's about the other factors (namely the file extension).

Okay, I see what you're saying now. I can agree with that.

 

And, no, embedding code in an image structure is not difficult at all.

No, difficult part is figuring out how to execute it. You can embed PHP in an image all day long, but unless you find a vulnerability within the application that treats the image like PHP, nothing will come of it.

[Jacques1]

So we agree that our goal is to prevent both server-side and client-side scripts from being executed, right?

 

My point is that MIME sniffing simply doesn't help with that:

• The PHP handler doesn't care about PNG signatures, it goes by the file extension (and of course it can be explicitly disabled). If the file ends with “.php”, then it will be executed, regardless or whether or not a PNG signature is present. If a file does not end with “.php”, then it will not be executed, regardless of whether or not a PNG signature is present. So the PNG signature is irrelevant.
• UAs generally don't care about PNG signatures either, they go by the Content-Type header. If a file is declared as an HTML document (or something similar), then any embedded code will be executed, regardless of whether or not a PNG signature is present. If a file is declared as an image, then embedded code will not be executed, regardless of whether or not a PNG signature is present. There are some edge cases like a missing Content-Type header or legacy IEs doing shenigans. But then server-side MIME sniffing still won't help, because the UA may come to an entirely different conclusion (hence the problem of chameleon files). So again the PNG signature is irrelevant.

No matter how you look at it, the theoretical benefit of server-side MIME sniffing is extremely low. It really only helps in the following scenario:

• You've screwed up by not proving a Content-Type header.
• The client has screwed up by using some IE fossile.
• The attacker is too dumb to exploit MIME detection weaknesses.

In this edge case of an edge case of an edge case, MIME sniffing might actually help, which is why I haven't written it off entirely. But I think we should care more about real solutions to real cases.

 

Hardening the server configuration and getting a subdomain is much more useful than an entire PHP script full of MIME checks.

Edited October 13, 2015 by Jacques1

[scootstah]

So we agree that our goal is to prevent both server-side and client-side scripts from being executed, right?

Indeed. I've always done both really - check MIME type, but explicitly set file extension when saving. I suppose setting the file extension is the important part.