image uploading.... my site got exploited

[turkman]

i wrote a site and was trying to make a site where images could be uploaded like an imageboard... i knew it was scrappy and probably had bugs so asked some people to try and exploit it which they did within minutes... they had uploaded exe's and executed php scrips etc...

 

they said they faked mime types - im not really sure what that means... i just need a fullproof way of checking that the content really is an image

 

this is what i was using

f ((($_FILES["image"]["type"] == "image/gif")
|| ($_FILES["image"]["type"] == "image/jpeg")
|| ($_FILES["image"]["type"] == "image/pjpeg")
|| ($_FILES["image"]["type"] == "image/bmp")
|| ($_FILES["image"]["type"] == "image/png"))
&& ($_FILES["image"]["size"] < 3000000))
{

$dt = date('Hismdy');
$ns = $dt.$_FILES["image"]["name"];
$_FILES["image"]["name"] = $ns;

if (file_exists("images/" . $_FILES["image"]["name"]))
{
echo $_FILES["file"]["name"] . " already exists. ";
}
else
{
move_uploaded_file($_FILES["image"]["tmp_name"],
"images/" . $_FILES["image"]["name"]);

}

 

[PFMaBiSmAd]

ALL the external information that your script received, even uploaded file information, is under the control of whoever or whatever submitted that information.

 

The safest way to prevent exploitation in an upload script is to insure that the location where you move the uploaded files to does not have permission to execute any scripts or programs. You can also prevent direct HTTP requests to the folder where the files are moved to and only output them indirectly through a .php script.

[turkman]

ok... thanks for that, i was led to believe there was a getimagesize function that could somehow stop it. i've been looking at the info for it and i cant work out how though.

 

so i need to set the image folder permissions to read only?

[turkman]

would that be 644 ?

[mattal999]

In addition to the previous suggestion, you should check the extension of the file being uploaded as they cannot rename it once it has been uploaded. Note this code is untested:

 

$validExtensions = array("png", "gif", "jpeg", "jpg", "bmp");
if ((($_FILES["image"]["type"] == "image/gif")
|| ($_FILES["image"]["type"] == "image/jpeg")
|| ($_FILES["image"]["type"] == "image/pjpeg")
|| ($_FILES["image"]["type"] == "image/bmp")
|| ($_FILES["image"]["type"] == "image/png"))
&& ($_FILES["image"]["size"] < 3000000) 
&& in_array(strtolower(end(explode(".", $FILES['image']['name']))), $validExtensions))
{

$dt = date('Hismdy');
$ns = $dt.$_FILES["image"]["name"];
$_FILES["image"]["name"] = $ns;

if (file_exists("images/" . $_FILES["image"]["name"]))
{
echo $_FILES["file"]["name"] . " already exists. ";
}
else
{
move_uploaded_file($_FILES["image"]["tmp_name"],
"images/" . $_FILES["image"]["name"]);

}

[turkman]

thanks for the help guys... i was really surprised how easy it was to mess up a website

[turkman]

mattall your code runs ok... however when i upload a genunie image it believes its not an image...

 

i assume its to do with this line

in_array(strtolower(end(explode(".", $FILES['image']['name']))), $validExtensions))

 

but im not really sure about whats happening here... is there a way of doing this so it can print out what its comparing so i can see where its going wrong?

[turkman]

i changed it to this, now it works

 

$validExtensions = array("png", "gif", "jpeg", "jpg", "bmp");
                                       $ext = end(explode(".",$_FILES["image"]["name"]));
                                       $ext2= strtolower($ext);
                                       add_notice("Extension was $ext");
                                        if ((($_FILES["image"]["type"] == "image/gif")
                                        || ($_FILES["image"]["type"] == "image/jpeg")
                                        || ($_FILES["image"]["type"] == "image/pjpeg")
                                        || ($_FILES["image"]["type"] == "image/bmp")
                                        || ($_FILES["image"]["type"] == "image/png"))
                                        && ($_FILES["image"]["size"] < 3000000)
                                        && in_array($ext2,$validExtensions))
                                        {