phpQuestioner Posted August 1, 2007 Share Posted August 1, 2007 I am trying to create a validation for my upload script that will only allow: "jpg" and "JPG" files to be uploaded. My script is below; can someone tell me what I am doing wrong and how to fix it - please? <? if (($_FILES[images][type] != "image/jpeg") || ($_FILES[images][type] != "image/pjpeg")&& ($_FILES[images][size] > 20000)) { echo "File Upload Failed - Please Upload JPG Or JPEG Files Only"; exit; } while(list($key,$value) = each($_FILES[images][name])) { if(!empty($value)) { $did = stripslashes($_POST['dealer']); $filename = $value; $add = "dealer-pics/$did/$filename"; //echo $_FILES[images][type][$key]; // echo "<br>"; copy($_FILES[images][tmp_name][$key], $add); chmod("$add",0777); } } echo "File/s Have Successfully Been Uploaded"; ?> Quote Link to comment Share on other sites More sharing options...
lightningstrike Posted August 1, 2007 Share Posted August 1, 2007 Your current script is insecure. Anyone could just upload a file called image.php.jpg which would contain php code they code maliciously execute on your server. Check the file extension firstly. Not to mention by using this line to give it 777 permissions chmod("$add",0777); They could easily delete your folder, or without open_basedir the entire webroot. Quote Link to comment Share on other sites More sharing options...
phpQuestioner Posted August 1, 2007 Author Share Posted August 1, 2007 ok - how would I go about doing that (not sure exactly what you mean) ? Quote Link to comment Share on other sites More sharing options...
lightningstrike Posted August 1, 2007 Share Posted August 1, 2007 1. Why would you chmod an abritrary file to 777? 2. Simple check the filename your uploading to see that it does not contain .php in it... 3. If you have the GD library you could perform additional checks by attempting to modify the image, suppressing the error then if it failed reply the image is invalid. (http://www.php.net/gd for more on this library) e.g. @$src_img=imagecreatefromjpeg($file); if ($src_img){ //valid JPEG }else{ //delete unlink($file); die("bad image..."); } 4. Avoid using short-open tags in php e.g. <? use <?php for portability reasons. Quote Link to comment Share on other sites More sharing options...
phpQuestioner Posted August 1, 2007 Author Share Posted August 1, 2007 This should take care of php file hacking; I believe: if (($_FILES[images][type] == "text/php")) { echo "File Upload Failed - Please Upload JPG Or JPEG Files Only"; exit; } But I still do not see while this does not work: if (($_FILES[images][type] != "image/jpeg") || ($_FILES[images][type] != "image/pjpeg")&& ($_FILES[images][size] > 20000)) { echo "File Upload Failed - Please Upload JPG Or JPEG Files Only"; exit; } Quote Link to comment Share on other sites More sharing options...
lightningstrike Posted August 1, 2007 Share Posted August 1, 2007 No your method will not take care of the php hacking. A user could upload a valid JPEG image as image.php.jpg but at the end it will have php code, undetectable to MIME type CHECK THE NAME of the file. e.g. <?php if (stristr($filename,".php")==true){ die("php extension banned..."); } ?> Quote Link to comment Share on other sites More sharing options...
phpQuestioner Posted August 1, 2007 Author Share Posted August 1, 2007 ok; how - I would have to use some type of trim or regex - right? Quote Link to comment Share on other sites More sharing options...
phpQuestioner Posted August 1, 2007 Author Share Posted August 1, 2007 sorry I did not see your code - I see it now....... I will give it a try Quote Link to comment Share on other sites More sharing options...
phpQuestioner Posted August 1, 2007 Author Share Posted August 1, 2007 So I tried this: <?php while(list($key,$value) = each($_FILES[images][name])) { if(!empty($value)) { if (stristr($filename,".php")==true){ die("php extension banned..."); } $did = stripslashes($_POST['dealer']); $filename = $value; $add = "dealer-pics/$did/$filename"; //echo $_FILES[images][type][$key]; // echo "<br>"; copy($_FILES[images][tmp_name][$key], $add); chmod("$add",0777); } } echo "File/s Have Successfully Been Uploaded"; ?> And This..... <?php while(list($key,$value) = each($_FILES[images][name])) { if (stristr($filename,".php")==true){ die("php extension banned..."); } if(!empty($value)) { $did = stripslashes($_POST['dealer']); $filename = $value; $add = "dealer-pics/$did/$filename"; //echo $_FILES[images][type][$key]; // echo "<br>"; copy($_FILES[images][tmp_name][$key], $add); chmod("$add",0777); } } echo "File/s Have Successfully Been Uploaded"; ?> And This.... <?php if (stristr($filename,".php")==true){ die("php extension banned..."); } while(list($key,$value) = each($_FILES[images][name])) { if(!empty($value)) { $did = stripslashes($_POST['dealer']); $filename = $value; $add = "dealer-pics/$did/$filename"; //echo $_FILES[images][type][$key]; // echo "<br>"; copy($_FILES[images][tmp_name][$key], $add); chmod("$add",0777); } } echo "File/s Have Successfully Been Uploaded"; ?> But none of the scripts above would stop a file with a PHP extension from being uploading to the directory. Plus if the file is something like "image.php.jpg"; the extension would be jpg - right; so the code: if (stristr($filename,".php")==true){ die("php extension banned..."); } would not work, because the extention would not be "PHP"; it would be "jpg". Quote Link to comment Share on other sites More sharing options...
phpQuestioner Posted August 1, 2007 Author Share Posted August 1, 2007 ok - how would I create one easy validation for preventing any and all MIME types from uploading to the directory; other then JPG and jpg files? Quote Link to comment Share on other sites More sharing options...
lightningstrike Posted August 1, 2007 Share Posted August 1, 2007 Please go to http://www.php.net and read more on string handling functions. The code i gave you WILL WORK using stristr. if (stristr($filename,".php")==true){ die("php extension banned..."); } stristr is case-insensitive it matches .php, .PhP, .pHP, etc. stristr searches through the whole filename so it WILL find .php in image.PhP.JPEG To only allow JPEG simply only allow JPEG, PJPEG, JPG MIME TYPES. Also check the extension. rename the file to something you want e.g. randomnumber.JPEG don't let the user name the file, or avoid bad extensions like .php validate it with the GD library mentioned earlier and you should be fine. Quote Link to comment Share on other sites More sharing options...
phpQuestioner Posted August 1, 2007 Author Share Posted August 1, 2007 Yeah I figured our where to put the code in the script to make it work. I also went to www.php.net and searched for the function and read about it. Thanks For Your Help. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.