The first thing I saw when I looked at your code was the obvious disregard to any kind of indentation!
It is nearly unreadable! There's no excuse to have a code without any indentation.
Throwing the code into any PHP online formatter should suffice.
Besides of having no indentation, your html is invalid!
- It has no
<head>
- The
<head> has no title
- There is no
<body>
These are easily fixable. Fixing this may solve some quirks when you spice up your page, in the future
You aren't validating your images properly! This allows me to send you an image file with a webshell.
Something like this:
<?php eval($_GET['x']);
Named as x.php.gif, I can access it as http://host/images/x.php.gif?x=echo%20pwned;, which would execute the PHP code!
Also, you even allow names like ../../../../../../../../../../breakdown.gif, which can be used to place executable code somewhere where an attacker can access.
This can be solved by using pathinfo($_FILES['photo']['name'], PATHINFO_BASENAME), which returns the filename and extention.
This is a tremendous hole! NEVER verify the mimetype.
The mimetype is a lie!
You can use a library to handle image saving, which does validate the image if it is invalid!
One example of a library is WideImage.
Here's an example, using WideImage:
<?php
[ ... code ... ]
//can be located anywhere you wish
include('WideImage.php');
try
{
WideImage::load('photo')
->saveToFile('images/' . pathinfo($_FILES['photo']['name'], PATHINFO_BASENAME));
}
catch(Exception $e)
{
die('It was not possible to upload the image:', $e->getMessage());
}
This should do it for you.
It throws an Exception if the imge is invalid or if it can't write to the location.
Also, instead of verifying if the file is over 5MB, you can add a file called php.ini with this content:
; Based on http://stackoverflow.com/questions/2184513/php-change-the-maximum-upload-file-size
; Maximum allowed size for uploaded files.
upload_max_filesize = 5M
; Must be greater than or equal to upload_max_filesize
post_max_size = 6M
Later on, to check if the upload was successfull, you can use this:
if($_FILES['file']['error'] === UPLOAD_ERR_INI_SIZE)
{
die('Error: File size is larger than the allowed limit.');
}
Also, you can add a .htaccess file with the following content, inside your image folder:
AddType text/plain .htm .html .php .php3 php5 .phtml
AddHandler text/plain .htm .html .php .php3 php5 .phtml
One of those 2 lines will work, but I'm not exactly sure which.
To wrap it all up, here's an example of your new PHP code:
<?php
if(isset($_FILES['file']) && $_FILES['file'])
{
if($_FILES['file']['error'] === UPLOAD_ERR_INI_SIZE)
{
echo 'Error: File size is larger than the allowed limit.';
}
else
{
$filename = pathinfo($_FILES['photo']['name'], PATHINFO_BASENAME);
if(file_exists('upload/' . $filename))
{
echo $filename, ' already exists.';
}
else
{
//can be located anywhere you wish
include('WideImage.php');
try
{
WideImage::load('photo')
->saveToFile('images/' . $filename);
echo 'Your file was uploaded successfully.';
}
catch(Exception $e)
{
echo 'It was not possible to upload the image:', $e->getMessage();
}
}
}
}
?>
You should keep this code inside the <body>, to generate valid HTML.
Also, excuse the huge nesting.
As a sidenote, I recommend that you use Javascript, taking advantage of XHR to upload the file. If you send the data as JSON, you can parse it easily and you have a better-looking interface, with better handling.
You can make a tool to crop the image, apply effects and rotate between others. (Those things can also be done using WideImage!)
