kiksy Posted October 27, 2009 Share Posted October 27, 2009 Hello, first post , and asking for help im afraid. Very new to PHP, was making good progress I thought , but im stuck with this part. I have a uploading form that works perfectly and uploads everything to "uploads/images" . What Im trying to do is upload .txt or .rtf files to "uploads/text" and .jpg and .gif's to "uploads/images . I thought this wasnt going to be too difficult but im totally stuck and have spent hours trying to solve this. If anyone could help at all and show my what I'm doing wrong, Id be very very grateful! This is the code as it stands. Right now, it ALWAYS goes to "uploadfail.php" and the file does not upload. If I remove all the code relating to filetype then both the text and image files succesully upload and I go to "uploadsucess.php" <? require('connection.php'); //my_sql_real..... makes it safe against mysql injection $title = mysql_real_escape_string($_POST['title']); $date = mysql_real_escape_string($_POST['date']); $author = mysql_real_escape_string($_POST['author']); $description = mysql_real_escape_string($_POST['description']); $type = mysql_real_escape_string($_POST['type']); $imgName = $_FILES['pic']['name']; $imgTmp = $_FILES['pic']['tmp_name']; $imgSize = $_FILES['pic']['size']; $imgType = $_FILES['pic']['type']; $maxFileSize = 200000; if ($imgType == "image/jpeg" || $imgType == "image/gif" || $imgType == "application/rtf" || $imgType == "application/x-rtf" || $imgType == "text/richtext" || $imgType == "text/plain" ) { $error = ""; } else { $error = "type"; } if ($imgType == "image/jpeg" || $imgType == "image/gif" ) { $filetype = "image"; } if ($imgType == "application/rtf" || $imgType == "application/x-rtf" || $imgType == "text/richtext" || $imgType == "text/plain" ) { $filetype = "text"; } else { $filetype = "other"; } if ($imgSize > $maxFileSize) { $error = "size"; } if ($error == "" && $imgName != "" && $filetype == "image" ) { $filePath = "uploads/images/".$imgName; move_uploaded_file($imgTmp, $filePath); if ($error == "" && $imgName != "" && $filetype == "text" ) { $filePath = "uploads/text/".$imgName; move_uploaded_file($imgTmp, $filePath); //another securtiy measure sprintf $query = sprintf("INSERT INTO gallery(title,date,author,type,description,path) VALUES ('%s' , '%s' ,'%s' ,'%s' ,'%s' ,'%s' )", $title,$date,$author,$type,$description,$filePath); if (!mysql_query($query, $connect)) { die(); } else { header("Location: uploadsuccess.php"); } } } else { header("Location: uploadfail.php"); } ?> Quote Link to comment Share on other sites More sharing options...
WolfRage Posted October 27, 2009 Share Posted October 27, 2009 The file type is not reliable at all. What you should be checking is the extension of the file name. Use explode on the file name and then use reverse array on the resulting array. Then check the first element in that resulting array agianst a white list of extensions. Quote Link to comment Share on other sites More sharing options...
kiksy Posted October 27, 2009 Author Share Posted October 27, 2009 Hi thanks for the quick reply. my $filetype works as Ive tested it with an echo at the end and it works. If I upload an image file , then my $filetype is "Image", if I upload a text file then my $filetype is "text". All I need to know is how to get this part of the code to work . if ($error == "" && $imgName != "" && $filetype == "image" ) { $filePath = "uploads/images/".$imgName; move_uploaded_file($imgTmp, $filePath); if ($error == "" && $imgName != "" && $filetype == "text" ) { $filePath = "uploads/text/".$imgName; move_uploaded_file($imgTmp, $filePath); Im pretty new to PHP , and youve completely lost me with your response. If you could elaborate on what you mean by "use explode" that would be great. Thanks again. Quote Link to comment Share on other sites More sharing options...
WolfRage Posted October 27, 2009 Share Posted October 27, 2009 I understand that you think your detection method works, but it is not fool proof. A user can modify a subimited file type to say whatever like image and really upload an exe. Then they can access the exe on your server and launch an application of there choosing. Of course exe do not work on Linux unless WINE is installed and even then are mostly harmless, but .deb and .sh files do work on linux and could also be faked. So please use explode to make sure the file is really the type it cliams to be. As for how to implement: (Note: I changed your naming convention some, you can change it back or make the corrections to your script. But I felt this made the names more universal to there respectful roles with in the script.) <? $array=explode('.',$name); //Here I am bring the name appart by each "." to seperate the name of the file from it's extension or filetype. $array=array_reverse($array); //Here we reverse the array so that multiple periods can not confuse the extension of the file. $ext=$array[0]; //The first value in the array is now the actual file extension. if ($error == "" && $name != "" && ($ext == "jpg" || $ext=='gif' || $ext=='png')) { //Now just check for each file extension you wish to accept. $filePath = "uploads/images/".$name; move_uploaded_file($temp, $filePath); } elseif ($error == "" && $name != "" && ($ext == "txt" || $ext=='rtf') ) { $filePath = "uploads/text/".$name; move_uploaded_file($temp, $filePath); } else { unset($temp); //here we delete the uploaded file because it did not pass our test. } ?> There is a faster method that uses posix to identify the file extension but I can not remember it right now, plus I trust this method because it is simple but works. Quote Link to comment Share on other sites More sharing options...
kiksy Posted October 28, 2009 Author Share Posted October 28, 2009 WOW! thank you so much! Works perfectly. Thanks too for the comments, I now understand explode and exactly why this works. Security isnt really an issues for this site as it will be running in a pretty closed environment, but thanks for the info, its always good to learn best practices. Once again, thanks so much for your time and knowledge. Quote Link to comment Share on other sites More sharing options...
WolfRage Posted October 28, 2009 Share Posted October 28, 2009 No problem and glad I could 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.