Help w/ upload site - Webmaster Forum - Web Design, Programming and SEO forums

05-20-2008, 02:17 AM

Xphic

Status: I love this place
Join date: Sep 2007
Location:
Expertise:
Software:

Posts: 523

iTrader: 13 / 100%

Xphic is on a distinguished road

Help w/ upload site

Alright befor I made imgtags, I used imgsync. I made a phpld design before, and uploaded to imgtags. But it didn't accept it because the term php was used.

Well today, I did the image check for imgtags. And noticed I have a .php.jpg. Avast responded saying it was a virus, so I deleted it. Now I want to know what's the bad thing about it?

If it's something extreme I guess i need to add a != in the code

05-20-2008, 03:46 AM

CreativeLogic

Status: Request a custom title
Join date: Feb 2005
Location:
Expertise:
Software:

Posts: 1,078

iTrader: 6 / 100%

CreativeLogic is on a distinguished road

Depending on how the upload script is wrote it may execute the code. The uploaded file may not actually be an image file but a php file. If the code is not wrote properly you could have that script do quite a bit of damage to your site.

05-20-2008, 04:25 AM

JulesR

Status: Member
Join date: Apr 2008
Location:
Expertise:
Software:

Posts: 129

iTrader: 0 / 0%

JulesR is on a distinguished road

This is exactly why people need to learn PHP security. In any image uploading site that's written in PHP you need to check the mime type of the file that's being uploaded, and NOT the file extension. File extension is completely meaningless because it's essentially user generated data and as such cannot be trusted.

05-20-2008, 12:26 PM

Xphic

Status: I love this place
Join date: Sep 2007
Location:
Expertise:
Software:

Posts: 523

iTrader: 13 / 100%

Xphic is on a distinguished road

okay, what do you guys think of this

PHP Code:


			
if ($HTTP_POST_FILES['file']['size'] < $maxfilesize){

### File under max file size



$rand = rand(1,9999);



if ($HTTP_POST_FILES['file']['type'] == "image/jpeg" || $HTTP_POST_FILES['file']['type'] == "image/pjpeg" || $HTTP_POST_FILES['file']['type'] == "image/gif" || $HTTP_POST_FILES['file']['type'] == "image/png" || $HTTP_POST_FILES['file']['type'] == "image/x-png")

{

### File is JPG, JPEG, PNG or GIF



$valid_filename = "/^[A-Za-z0-9_-\s]+\.(jpg|gif|png)$/";

if (preg_match($valid_filename, $_FILES['file']['name']) && (strlen($_FILES['file']['name']) < 255))

{



if (!file_exists($uploaddir."/tn"))

{

mkdir($uploaddir."/tn", 0700);

chmod($uploaddir."/tn", 0777);

}



if (is_file($uploaddir."/".$_FILES['file']['name'])){

### File already exists, rename and upload



$newfile = $rand.strtolower($_FILES['file']['name']);



move_uploaded_file($_FILES['file']['tmp_name'],"".$uploaddir."/".$newfile);



if (file_exists($uploaddir."/".$newfile))

{

createthumb($uploaddir."/".$newfile,$uploaddir."/tn/".$newfile,140,120);

}

$error = "0";



} else {

### One of a kind file, upload



$newfile = $_FILES['file']['name'];



move_uploaded_file($_FILES['file']['tmp_name'],$uploaddir."/".$_FILES['file']['name']);



if (file_exists($uploaddir."/".$newfile))

{

createthumb($uploaddir."/".$newfile,$uploaddir."/tn/".$newfile,140,120);

}



$error = "0";



}



### Success, file was uploaded

include('success.php');

}

else

{

$error = "Filename invalid. Please rename your file and upload it again.";

}



} else {

### File does not match allowed types



$error = "Your file is not an acceptable file type";



}



} else {

### File to large



$error = "Your file is too large";



}



}

Is there security flaws in that code

05-20-2008, 05:13 PM

JulesR

Status: Member
Join date: Apr 2008
Location:
Expertise:
Software:

Posts: 129

iTrader: 0 / 0%

JulesR is on a distinguished road

Glaringly obvious issues:

1. $HTTP_POST_FILES is deprecated. Use $_FILES instead.
2. If checking the mime type you don't need the following:

PHP Code:


			
$valid_filename = "/^[A-Za-z0-9_-\s]+\.(jpg|gif|png)$/";
if (preg_match($valid_filename, $_FILES['file']['name']).....

3. This will break on an SuPHP configured server:

PHP Code:


			
if (!file_exists($uploaddir."/tn"))
{
mkdir($uploaddir."/tn", 0700);
chmod($uploaddir."/tn", 0777);
}

Check the directory is writeable before trying to force 777 permissions on a folder. If ownership is set correctly you don't need 777 permissions.

05-21-2008, 09:00 PM

Xphic

Status: I love this place
Join date: Sep 2007
Location:
Expertise:
Software:

Posts: 523

iTrader: 13 / 100%

Xphic is on a distinguished road

Okay, and I have added in the piece of code, thanks more the help. Much appreciated