File Upload - Security - Is this image uploader secure?

[BMR777]

Hello All,

 

I'm working on a script where registered users should be able to upload either a .gif or .jpg file to the server.  I have made a file upload script and I was hoping some of you could look over it for security and tell me if there is anything that is not secure about it or anything I could add to it to increase security and prevent against users adding malicious files to the server.

 

Here's the script:

 

<?php

// Wake the sleeping giant
include("inc/functions.php");
connect();
$themeurl = themeurl();
$site_title = sitetitle();
$site_name = sitename();
$slogan = slogan();
$newsbar = newsbar();


if($newsbar != ""){
$shownews = "<div class='subheader'><p>".$newsbar."</p></div>";
}
else{
$shownews = "<div class='subheader'></div>";
}

// **********************************************************************
// Check if user is logged in
// **********************************************************************

$userdata = logincheck();
$isloggedin = $userdata[loginstatus];
$loggedinname = $userdata[username];

// **********************************************************************
// We do all our prepwork here
// **********************************************************************

// If we're not logged in, we cannot access this page...

if($isloggedin != "yes"){
$article_title = "403 Forbidden";
$article_content = "You do not have permission to access the file uploads page.  Only artists may access this page.
Are you an artist?  <a href='login.php'>Log in</a> or <a href='register.php'>register</a> to upload files.";

}
else{

// BEGIN FILE UPLOAD

$flag = 0; // Safety net, if this gets to 1 at any point in the process, we don't upload.

$filesize = $_FILES['uploadedfile']['size'];
$mimetype = $_FILES['uploadedfile']['type'];
$filename = $_FILES['uploadedfile']['name'];


$filename = htmlentities($filename);
$filesize = htmlentities($filesize);
$mimetype = htmlentities($mimetype);

//Default upload directory
$uploaddir = "picuploads/gif";


//***************************************************************************
//First we determine if the file is a gif or a jpg by checking the extension

//First check and see if the file is a .gif file
$isgif = "no";

$whitelist = array(".gif");
foreach ($whitelist as $ending) {

if(substr($filename, -(strlen($ending))) != $ending) {

//File is not a gif file, so let's do nothing right now
//When we check for if it is a jpg we will flag the file

}
else{
//The file IS a gif file, so we set the isgif to true
$isgif = "yes";

}
}

// Now we check if it is a .jpg file or not, because it is not a gif

if($isgif != "yes"){

$whitelist = array(".jpg");
foreach ($whitelist as $ending) {

if(substr($filename, -(strlen($ending))) != $ending) {

if($flag == 0){
$error = "The file type or extention you are trying to upload is not allowed!  
You can only upload gif or jpg files to the server!";
}

$flag++;
}
}

}

//***************************************************************************


/*
if($filename != ""){

echo "Beginning upload process for file named: ".$filename."<br>";
echo "Filesize: ".$filesize."<br>";
echo "Type: ".$mimetype."<br><br>";

}

*/

//First generate a MD5 hash of what the new file name will be
//Force a file extention on the file we are uploading

//Now we create a hashed file name of the file and set the upload directory...

if($isgif == "yes"){

$date = date('Y-m-d');
$hashstring = $filename."_".$date;

$hashedfilename = md5($hashstring);
$hashedfilename = $hashedfilename.".gif";
$target_path = "picuploads/gif/";
$uploaddir = "picuploads/gif";

}
else if ($isgif == "no" and $flag == 0){

//File is a jpg

$date = date('Y-m-d');
$hashstring = $filename."_".$date;

$hashedfilename = md5($hashstring);
$hashedfilename = $hashedfilename.".jpg";
$target_path = "picuploads/jpg/";
$uploaddir = "picuploads/jpg";

}


//SET TARGET PATH?
$target_path = $target_path . basename( $filename );


//Check for empty file
if($hashedfilename == ""){

if($error == ""){
$error = "No File Exists!";
}

$flag = $flag + 1;

}

//Now we check that the file doesn't already exist.
$existname = $uploaddir."/".$hashedfilename;

if(file_exists($existname)){

if($flag == 0){
$error = "Your file already exists on the server!  
Please choose another file to upload or rename the file on your 
computer and try uploading it again!";
}

$flag = $flag + 1;
}

////////////////////////////////////////////////////////////////////////////////////////////////////
////////////////////////////////////////////////////////////////////////////////////////////////////


//Now we check the filesize.  If it is too big then we reject it


if($filesize > 153600){
//File is too large

if($flag == 0){
$error = "The file you are trying to upload is too large!  Files must be under 150 KB.";
}

$flag = $flag + 1;
}

////////////////////////////////////////////////////////////////////////////////////////////////////
////////////////////////////////////////////////////////////////////////////////////////////////////

//Check the mimetype of the file
if($mimetype != "image/gif" and $mimetype != "image/jpeg"){

if($flag == 0){
$error = "The file you are trying to upload does not contain expected data.
Are you sure that the file is a .gif or .jpg file?";
}

$flag = $flag + 1;
}

////////////////////////////////////////////////////////////////////////////////////////////////////
////////////////////////////////////////////////////////////////////////////////////////////////////

//Now that we checked the mime type client side, let's check it again server side...

$imageInfo = getimagesize($_FILES["uploadedfile"]["tmp_name"]); // note that we need to use the temporal name since it has not yet been moved  
if($imageInfo["mime"] != "image/gif" and $imageInfo["mime"] != "image/jpeg")  
{  
if($error == ""){
$error = "The file you are trying to upload does not contain expected data.
Are you sure that the file is a .gif or .jpg file?";
}

$flag++;

} 


////////////////////////////////////////////////////////////////////////////////////////////////////
////////////////////////////////////////////////////////////////////////////////////////////////////
////////////////////////////////////////////////////////////////////////////////////////////////////
////////////////////////////////////////////////////////////////////////////////////////////////////

//All checks are done, actually move the file...

if($flag == 0){

if(move_uploaded_file($_FILES['uploadedfile']['tmp_name'], $uploaddir."/".$hashedfilename)) {
    

if(@file_exists($uploaddir."/".$hashedfilename)){

$article_title = "Success!";
/*

$article_content = "The file ".  basename( $filename ). " 
      has been uploaded.  Your file is <a href='uploads/$uploaddir/$hashedfilename'>here</a>.";

*/

$article_content = "The file ".  basename( $filename ). " 
      has been uploaded successfully!  It will now appear on your profile and in your photo gallery.";

}	
else{
      $article_title = "ERROR!";
$article_content = "There was an error uploading the file, please try again!";

}


} else{
    $article_title = "ERROR!";
    $article_content = "There was an error uploading the file, please try again!";

    

}

}
else {
$article_title = "ERROR!";
if($error != ""){
$article_content = $error;
}
else{
$article_content = "File Upload Failed!";
}
}

//More code here
//Done with upload, so insert data into database



$origfilename = secure(basename( $filename ));
$hashedfilename = secure($hashedfilename);
$location = $uploaddir."/".$hashedfilename;

//*******************************************************

$info = $_POST['info'];
$info = secure($info);

//*******************************************************

if($flag == 0){

$crdate = date('Y-m-d');
mysql_query("INSERT INTO picsmap VALUES ('', '$loggedinname', '','$location','$location', 'profileimage', '$crdate', '$info')");

}


$article_content = $article_content."<br><br><u>What do you want to do now?<br><br>
<a href='uploadpicform.php'>Upload another picture file</a><br>
<a href='managepicuploads.php'>Manage uploads or change / delete file info</a><br>
<a href='account.php'>Manage My Account</a>";


}





// **********************************************************************
// End Prepwork - Output the page to the user
// **********************************************************************

//Define our current theme
$file = $themeurl;

// Do the template changes and echo the ready template
$template = file_get_contents($file);
//$template = replace(':SITETITLE:',$site_title,$template);
$template = replace(':SITENAME:',$site_name,$template);
$template = replace(':SLOGAN:',$slogan,$template);
$template = replace(':ARTICLETITLE:',$article_title,$template);
$template = replace(':ARTICLEDATE:',$article_date,$template);
$template = replace(':ARTICLECONTENT:',$article_content,$template);
$template = replace(':LINK1:',$link1,$template);
$template = replace(':LINK2:',$link2,$template);
$template = replace(':LINK3:',$link3,$template);
$template = replace(':NEWSBAR:',$shownews,$template);

/*

//Ad Management
$header = @file_get_contents("ads/header.txt");
$footer = @file_get_contents("ads/footer.txt");
$tower = @file_get_contents("ads/tower.txt");

$header = stripslashes($header);
$footer = stripslashes($footer);
$tower = stripslashes($tower);

$template = replace(':HEADERAD:',$header,$template);
$template = replace(':FOOTERAD:',$footer,$template);
$template = replace(':TOWERAD:',$tower,$template);

*/

//**************************************************************

//Custom template replacement to allow the javascript to work properly

$oldtext = "<head>
<meta name=\"author\" content=\"Luka Cvrk (www.solucija.com)\" />
<meta http-equiv=\"content-type\" content=\"text/html;charset=iso-8859-2\" />
<link rel=\"stylesheet\" href=\"templates/default/images/style.css\" type=\"text/css\" />
<title>:SITETITLE:</title>
</head>";

$newtext = "<head><title>File Upload</title>
<script language=\"Javascript\">
<!-- Copyright 2001 Bontrager Connection, LLC
function WorkingMessage() {
   var url=\"\";       // Blank for thankyou page.
   var height = 100; // Height of popup
   var width = 450;  // Width of popup
   var att='width=' + width + ',height=' + height;
   WorkingMessagePopup=window.open(url,\"wmp\",att);
}
function KillWorkingMessagePopup(){
   WorkingMessage();
   WorkingMessagePopup.close();
} // -->
</script>	<meta name=\"author\" content=\"Luka Cvrk (www.solucija.com)\" />
<meta http-equiv=\"content-type\" content=\"text/html;charset=iso-8859-2\" />
<link rel=\"stylesheet\" href=\"templates/default/images/style.css\" type=\"text/css\" /></head>";

$template = replace($oldtext,$newtext,$template);


$oldtext = "<body>";

$newtext = "<body onLoad=\"KillWorkingMessagePopup();\">";

$template = replace($oldtext,$newtext,$template);

//**************************************************************

//Is the user logged in?

if ($isloggedin == "yes"){

$logincontent = logincontent($loggedinname);

$template = replace(':LOGINBAR:',$logincontent[loginbar],$template);
$template = replace(':WELCOMEORREGISTER:',$logincontent[welcome],$template);
$template = replace(':LOGINORACCT:', $logincontent[content] ,$template);
$friends = minifriends();
$template = replace(':FRIENDS:',$friends,$template);
}
else{

//User is not logged in
$template = replace(':LOGINBAR:','<b>You are not Logged in!</b> <a href="login.php">Log in</a>  or <a href="register.php">register</a> to start downloading music!',$template);
$template = replace(':WELCOMEORREGISTER:','<u>Member Login:</u>',$template);
$loginform = loginform();
$template = replace(':LOGINORACCT:', $loginform ,$template);
$friends = "";
$template = replace(':FRIENDS:',$friends,$template);
}

$morecontent = morecontent();
$template = replace(':MORECONTENT:',$morecontent,$template);

// **********************************************************************
// THIS IS THE LAST THING WE DO!
// **********************************************************************

echo $template;



?>

 

Any thoughts would be appreciated.  Thanks for looking.

 

Brandon

[printf]

Is this image uploader secure? No

 

Think about what your doing and you should realize you have lost focus on that task at hand. You don't do anything that tells me you understand anything related to securing your application. PHP by default gives you the basic tools to process uploads securely but you don't use any of them in the control order they should be used in. You also do a lot of ASSUMING, (assign variables that may not even exist) and that is something you should never do. Those are just the basic problems I see, but there are also many illogical control problems too. Also turn error reporting on. Just so you understand, what I am telling you is not to be mean, I am only trying to help you learn.

[BMR777]

Well printf,

 

Thanks.  Are there any checks then that you would recommend that I run on the uploaded file?  Basically my thinking is that if it fails any one of the checks I run, the script doesn't upload the file.

 

I've been reading various tutorials on the internet about safely uploading files in PHP and they all seem to have various methods of verifying the uploaded files.  Some say you should check the file extension, some say you should use getimagesize to determine if the file is really an image, so I have tried to take the best of all the tutorials and make a secure uploader.

 

Is there any check I should run on the uploaded file that I haven't already run on it?  From what I've been reading a lot of things like mimetype sent by the browser can be faked, so I use the server getimagesize() to check the mime type on the server end as part of the checks.  Can this be faked / bypassed as well and if so what can I do to prevent against this type of attack?  You mention basic tools of PHP to process uploads securely, so if you would be so kind as to point me in the right direction of what you mean I would appreciate it.

 

Thanks,

BMR777