Is This Secure Enough?

[Azercii]

I'm not amazing with PhP, so excuse me if it looks terrible xD I've taken tutorials, edited them to fit my wanting and tried it out, it seems to deny anything other than an image type, but could it be abused?

<div id="image-upload">
<h2>Upload your image</h2>
<form action="upload.php" method="post" enctype="multipart/form-data"> 
Upload:<br><br> 
<input type="file" name="image"><br><br>
Image Title:<br><br> 
<input type="text" name="image_title"><br><br> 
<input type="submit" name="submit" value="Upload"> 
</form>
	
	
<?php
include("upload_file.php");

function GetImageExtension($imagetype)
{
if(empty($imagetype)) return false;
switch($imagetype)
{
case 'image/bmp': return '.bmp';
case 'image/jpeg': return '.jpg';
case 'image/png': return '.png';
default: return false;
}
}
		 
			 
if ($_FILES['image']['error'] !== UPLOAD_ERR_OK) {
die();
}

$extension = getimagesize($_FILES['image']['tmp_name']);
if ($extension === FALSE) {
die("<br><font color='#8B0000'>Unable to determine image typeof uploaded file</font>");
}

if (($extension[2] !== IMAGETYPE_GIF) && ($extension[2] !== IMAGETYPE_JPEG) && ($extension[2] !== IMAGETYPE_PNG)) {
die("<br><font color='#8B0000'>Only images are allowed!</font>");
}
			 
			 
			 			 
if (!empty($_FILES["image"]["name"])) {

$file_name=$_FILES["image"]["name"];
$temp_name=$_FILES["image"]["tmp_name"];
$imgtype=$_FILES["image"]["type"];
$ext= GetImageExtension($imgtype);
$imagename=$_FILES["image"]["name"];
$target_path = "../../images/upload/".$imagename;
$title = $_POST["image_title"];

		
if(move_uploaded_file($temp_name, $target_path)) {
$query_upload="INSERT into `images_tbl` (`images_path`,`submission_date`,`image_title`) VALUES 
('".$target_path."','".date("Y-m-d")."','".$title."')";

mysql_query($query_upload) or die("error in $query_upload == ----> ".mysql_error()); 
echo '<br>Image uploaded!';
}else{
echo '<br><font color="#8B0000">Only images are allowed!</font>';
} 
}
?>

[fastsol]

In a word, NO, it's not secure at all.  There seems to be a lot of debate about how to secure file uploads from a client.  The important steps to help ensure it's safe to upload and actually use, generally require you to have full access to the server file system and are able to store files outside the root of the website.  That tends to be difficult cause most paid hosting places don't allow you such access.  So in that case all you can really do is the basic stuff and hope for the best. 

 

Here is a good tutorial on the subject https://www.youtube.com/watch?v=PRCobMXhnyw .  Basically your code is easily bypassed and should not be used on a live site.

[Azercii]

Currently have no speakers plugged in, thought the captions would help but I couldn't have been more wrong aha

 

Part of me wants to leave it as is, as I know they can't upload anything but an image file (no .txt, .html, .php, .js, etc) which for the standard user is fine. I need to work out how to do moderation of files via an admin panel, so that anything that isn't an image, or is deemed unsuitable, can be removed before being shown via the gallery. Problem is, if that one smart-*** does come along and bypasses it, I have no one to blame but myself.

Edited July 13, 2014 by Azercii

[Jacques1]

Your script doesn't have any security at all.

 

The problem is that you appearently don't understand the risks and how to deal with them. Yes, you're doing a bunch of type checks, but none of this has anything to do with security.

 

Throwing together a bunch of tutorials is not a good starting point. Most of them are already crap, and if people blindly copy and paste them, that's like Chinese whispers: At the end, it's all gibberish.

 

A much better approach is to actually think about the problem and come up with an intelligent solution. So what is the problem?

• A user might upload a server-side script to attack your server.
• Or they might upload a client-side script to attack other users or, indirectly, your server.

Validating the file type does not help against any of this. I can give you a perfectly valid image which even looks fine and still contains malicious code. For example, the JPEG format explicitly allows arbitrary text comments, so an attacker can easily embed PHP or JavaScript code.

 

The point is that the file must be treated as an image. As long as that's the case, there's no problem. But if your server or the browsers of your users think they should execute the file, you're in deep trouble.

 

How a file is treated depends mostly on the extension. Never accept the user-provided extension like you currently do. An attacker will upload an image ending with “.php” or “.html”, and suddenly you have a PHP script or HTML document on your server. Only allow specific extensions like “.jpeg” or “.png”.

 

You also need to choose a unique filename. You can't just take the user-provided name, because then your users may overwrite existing files. Either use the value of an AUTO_INCREMENT column or generate a random filename. The latter is preferred if you don't want the whole world to see who uploaded what when.

 

To make sure that your webserver will indeed not execute any uploaded files, turn off script execution in the upload directory. Apache has the SetHandler directive for that. To prevent client-side scripts from being executed, use Content Security Policy. This is limited to modern browsers, though. In addition to that, serve the uploaded files from a separate domain so that malicious script cannot affect your main site even if they are executed. Avoid using a subdomain, because then an attacker will still be able to set cookies for the main site and, for example, perform a session fixation attack. But if a subdomain is all you can get, it's better than nothing.

 

Wrapping it up:

• Validate the extension, not the file type. Only allow specific image extensions.
• Choose a unique filename, do not accept the user-provided name.
• Turn off script execution in the upload directory.
• Use Content Security Policy.
• Serve the uploaded files from a separate domain (preferrably not a subdomain of your main site).

[Azercii]

I can't seem to find tutorials that are actually secure, I really would like to learn how to secure uploads properly and the best way to go about it.

 

With naming the file, would it be better to combine it with the users ID? Not sure if it can be done like this, but maybe: 07-mynewsetup.jpg.

[Jacques1]

There isn't always a “tutorial” telling you exactly what to do. I've given you a fairly extensive overview, why not start with that? If you have issues with a specific step, just ask.

 

 

 

With naming the file, would it be better to combine it with the users ID? Not sure if it can be done like this, but maybe: 07-mynewsetup.jpg.

 

You could do that, but why would you? If the user has multiple files with the same name in different directories, they again run into the problem of name collisions. Either they have to manually rename their files or, which is even worse, they accidentally overwrite old files. In addition to that, this combination of the internal user ID and the user-provided name is neither convenient for you nor for the user. So what's the point of that?

 

The original filename chosen by the user and the actual filename on the server are two different things which have nothing to do with each other. When you store the file on your server, you don't care about which name the user chose on their PC. You need to choose a name which is appropriate for your server (no collisions, easy lookup etc.). The original filename is just metadata which goes into the database.

 

Either use the primary key from the database as the filename or a purely random name (which is what I prefer). Of course you can display the original filename on the page, but it makes no sense to actually store your files like that. If you absolutely must include the original filename the URL (for SEO purposes or whatever), append it to the physical name:

https://uploads.yoursite.com/images/8881d83fe82501a7adf9049714f0d20f-my-cute-kitten.jpg