Can anyone help me secure this script? I am stuck!

[thesecraftonlymultiply]

Hi everyone,

With the much needed help of 3 online tutorials I have managed to write a working PHP upload script.  However it is far from secure!  At minimum what I would like to do is restrict the file type to .jpg only and also restrict the file size to a maximum of 500KB.  I have had a good look around but am not getting anywhere on this and so am asking for help!

 

here is my script:

 

<?php
//  UPLOAD.PHP
//  UPLOAD FILE TO UPLOADEDIMAGES FOLDER ON SERVER
//  OPEN, DYNAMICALLY RESIZE AND OVERWRITE OLD FILE


//  MOVE UPLOADED FILE TO UPLOADED IMAGES DIRECTORY
move_uploaded_file($_FILES['Filedata']['tmp_name'], "./uploadedimages/".$_FILES['Filedata']['name']);

//  SET APPROPRIATE FILE PERMISSIONS
chmod("./uploadedimages/".$_FILES['Filedata']['name'], 0777);
echo(" ");

//  SELECT IMAGE READY FOR RESIZING
$uploadedimage = ("./uploadedimages/".$_FILES['Filedata']['name']);

//  CREATE AN IMAGE FROM IT IN PREPERATION FOR RESIZE
$imageready = @imagecreatefromjpeg($uploadedimage);

//  IF IMAGE CAN'T BE OPENED PRINT ERROR
if ($imageready === false) {
die ('Unable to open image');
}

//  OBTAIN ORIGINAL WIDTH AND HEIGHT
$width = imagesx($imageready);
$height = imagesy($imageready);

//  SET NEW WIDTH OF 600 AND CALCULATE APPROPRIATE HEIGHT - MAINTAIN ASPECT RATIO
$new_width = 600;
$new_height = ($height/$width) * 600;

//  RESIZE AND RESAMPLE IMAGE
$image_resized = imagecreatetruecolor($new_width, $new_height);
imagecopyresampled($image_resized, $imageready, 0, 0, 0, 0, $new_width, $new_height, $width, $height);

//  WRITE RESIZED IMAGE
$filename = "./uploadedimages/".$_FILES['Filedata']['name'];
imagejpeg($image_resized, $filename, 100);

//  CLEAN UP
imagedestroy($imageready);
imagedestroy($image_resized);


?>

 

I have been experimenting with using something like:

 

if ($_FILES['Filedata']['size'] > 500 )
{
die ("ERROR: Large File Size");

}

 

for the size restriction but am not getting anywhere.  Can anyone please help me?  Thankyou for reading.

 

Luke

[discomatt]

Manual is usually the best place to start.

 

For one, size always returned in bytes... so to limit to 500 kB (kilo =~ 1024 bytes) soo... 512000 bytes = 500 kilo Bytes

 

And to find the extension use

 

$ext = strtchr($_FILES['Filedata']['name'], '.');

if ( strtolower($ext) != 'jpg' && strtolower($ext) != 'jpeg' )

  exit('Bad file uploaded');

 

Additionally, you can check for mime-types, but because these can be masked, use it as a secondary check only.

 

if ( strtolower($_FILES['Filedata']['type']) != 'image/jpeg' && strtolower($_FILES['Filedata']['type']) != 'image/pjpeg' )

  exit('Bad file upload');

 

[Fadion]

discomatt, there's no strtchr() function. Guess u meant strrchr(). Eventhough in your code $ext would be printed as '.jpg' and not just 'jpg' like u checked it. I usually use:

 

<?php
$filename = $_FILES['Filedata']['name'];
$extension = strtolower(substr(strrchr($filename, '.'), 1));
if($extension == 'jpg' or $extension == 'jpeg'){
   //code here
} else{
   echo 'File extension is not supported.';
}
?>

[discomatt]

Bad typo. Should've tested. Sorry.

[thesecraftonlymultiply]

thankyou for the quick replies am working through the code now and gonna test it in a min

[thesecraftonlymultiply]

should this be working? ....

 

<?PHP
$imagename = $_FILES('Filedata']['name'];
$extension = strtolower(substr(strrchr($imagename, '.'), 1));;
if ($extension == 'jpg' or $extension == 'jpeg') && ($_FILES['Filedata']['size'] < 512000) {

MY CODE HERE***

} else {
echo 'error!';
}

 

 

it doesn't seem to be!  any help appreciated once again!

[Fadion]

U can try it by yourself , but yes it will work. My suggestion is to have different if statements for extension and filesize, so u show the users what they did wrong (like: extension not supported and filesize is too big) and not just inform them about the error.

[thesecraftonlymultiply]

I'll try separating them then but at the moment the above script I posted isn't working.. but it should be??

[thesecraftonlymultiply]

I am struggling... why is this not working?!

 

<?PHP
$imagename = $_FILES['Filedata']['tmp_name'];
$filesize = $_FILES['Filedata']['size'];
$extension = strtolower(substr(strrchr($imagename, '.'), 1));

if ($extension == 'jpg' or $extension == 'jpeg') && ($filesize < 52000) {

** MY UPLOAD CODE HERE**

} else {
echo 'File extension is not supported';
}

?>

[Fadion]

U have it $_FILES['Filedata']['tmp_name'] and im not sure tmp_name holds the correct extension. Just to be sure change it to $_FILES['Filedata']['name']. Also be sure to call the correct name of input file (Filedata with uppercase F), maybe it is just "filedata". The other part seems ok.