Page 1 of 1

How to make image uploads safe.

Posted: Sat Aug 20, 2011 10:06 am
by condoravenue1
Here is what I use to upload profile pictures to a folder. I know this isn't secure yet. The folder's permission is set to 777 because that is the only way I could make it work.

Let me know what needs to be added/changed to keep hackers away.

Thanks!

Code: Select all

<?php
session_start();
if(!isset($_SESSION['username'])){header("location: ../index.php"); exit();}

$con = mysql_connect("**","**","**");
mysql_select_db("**", $con);

$username = mysql_real_escape_string($_SESSION['username']);

// get extension
if ($HTTP_POST_FILES['uploadedfile']["type"] == "image/gif") {$ext = ".gif";}
if ($HTTP_POST_FILES['uploadedfile']["type"] == "image/png") {$ext = ".png";}
if ($HTTP_POST_FILES['uploadedfile']["type"] == "image/jpeg") {$ext = ".jpg";}

// validate
if ($HTTP_POST_FILES['uploadedfile']["type"] != "image/gif" && $HTTP_POST_FILES['uploadedfile']["type"] != "image/png" && $HTTP_POST_FILES['uploadedfile']["type"] != "image/jpeg") {$valid = "false"; $errors = $errors . "1";}
if ($HTTP_POST_FILES['uploadedfile']['size'] > 3000000) {$valid = "false"; $errors = $errors . "2";}

// if invalid, go to error page
if ($valid == "false"){
header("location: ../error_pages/upload_picture_error.php?why=$errors");
exit();}

// resize, rename, copy to folder
$path = "../profile_pictures/".$username . $ext;
$file_name = $HTTP_POST_FILES['uploadedfile']['tmp_name'];

copy($file_name, $path);
   include('SimpleImage.php');
   $image = new SimpleImage();
   $image->load($path);
   $image->resizeToWidth(100);
   $image->save($path);
$picture = $username . $ext;

$result = mysql_query("SELECT * FROM perm WHERE username = '$username'");
while($row = mysql_fetch_array($result)) 
{$old_picture = $row['picture'];}

// delete old default picture
unlink("../profile_pictures/".$old_picture);

mysql_query("UPDATE perm SET picture = '$picture' WHERE username = '$username'");

echo '<script type="text/javascript">window.location = "../account.php";</script>';

?>

Re: How to make image uploads safe.

Posted: Sat Aug 20, 2011 12:42 pm
by Christopher
The first think is to use move_uploaded_file() instead of copy() because it does security checks. See the manual for that function for more info about uploads. There is also a whole section of the manual about Handling file uploads - http://us.php.net/manual/en/features.file-upload.php

Re: How to make image uploads safe.

Posted: Sat Aug 20, 2011 9:00 pm
by condoravenue1
I had to set the permissions of the folder that the files are uploaded into to 777. Is this what it should be?

Re: How to make image uploads safe.

Posted: Sat Aug 20, 2011 9:26 pm
by Christopher
I would set the directory to the user that the web server runs as ... and then 700.

Re: How to make image uploads safe.

Posted: Sun Aug 21, 2011 9:23 am
by social_experiment

Re: How to make image uploads safe.

Posted: Tue Aug 23, 2011 3:16 am
by condoravenue1
As I said before, the folder's permission is 777. If I change it, not only does my uploading code not work, but the pictures don't display on the website. The way it is now, can someone upload files to that folder without using my website? My main concern is that someone will upload a bunch of junk to the folder, or overwrite someone else's picture with an unwanted one. Or perhaps they can delete other's photos.

Btw, the site is here: http://bible-help.com/

New Code:

Code: Select all

<?php
session_start();
if(!isset($_SESSION['username'])){header("location: ../index.php"); exit();}

$con = mysql_connect("*","*","*");
mysql_select_db("*", $con);

$username = mysql_real_escape_string($_SESSION['username']);


// get extension
if ($HTTP_POST_FILES['uploadedfile']["type"] == "image/gif") {$ext = ".gif";}
if ($HTTP_POST_FILES['uploadedfile']["type"] == "image/png") {$ext = ".png";}
if ($HTTP_POST_FILES['uploadedfile']["type"] == "image/jpeg") {$ext = ".jpg";}

// validate
if ($HTTP_POST_FILES['uploadedfile']["type"] != "image/gif" && $HTTP_POST_FILES['uploadedfile']["type"] != "image/png" && $HTTP_POST_FILES['uploadedfile']["type"] != "image/jpeg") {$valid = "false"; $errors = $errors . "1";}
if ($HTTP_POST_FILES['uploadedfile']['size'] > 3000000) {$valid = "false"; $errors = $errors . "2";}
if (strpos(strtolower($HTTP_POST_FILES['uploadedfile']['name']), '.php') !== false || strpos(strtolower($HTTP_POST_FILES['uploadedfile']['name']), '.phtml') !== false) {$valid = "false"; $errors = $errors . "1";}

// if invalid, go to error page
if ($valid == "false")
{
header("location: ../error_pages/upload_picture_error.php?why=$errors");

exit();
}

$result = mysql_query("SELECT * FROM perm WHERE username = '$username'");
while($row = mysql_fetch_array($result)) 
{$old_picture = $row['picture'];}
// delete old default picture
unlink("../profile_pictures/".$old_picture);

// resize, rename, copy to folder
$file_name = $HTTP_POST_FILES['uploadedfile']['tmp_name'];
$path = "../profile_pictures/".$username . $ext;

move_uploaded_file($file_name, $path);

   include('SimpleImage.php');
   $image = new SimpleImage();
   $image->load($path);
   $image->resizeToWidth(100);
   $image->save($path);

$picture = $username . $ext;

mysql_query("UPDATE perm SET picture = '$picture' WHERE username = '$username'");

echo '<script type="text/javascript">window.location = "../account.php";</script>';

?>
Christopher wrote:The first think is to use move_uploaded_file() instead of copy() because it does security checks.
Did that.

Re: How to make image uploads safe.

Posted: Tue Aug 23, 2011 5:52 am
by social_experiment
condoravenue1 wrote:The way it is now, can someone upload files to that folder without using my website? My main concern is that someone will upload a bunch of junk to the folder, or overwrite someone else's picture with an unwanted one. Or perhaps they can delete other's photos.
No, you need access to a file that allows for file uploading to your server (or access to the webserver like an FTP account). Same goes for deleting / overwriting. Just a point on overwriting, it will be better to create new file names, should there be an upload ability on your site, in that way, the overwriting issue is pretty much resolved.