Help with securing an upload form

[petezaman]

Hi all,

I have an image and text upload script which leads from a form where a user has been chosen from a combobox, this stores the link to the image and the text in a MySQL database. I have amended the form and script so that it works perfectly for files up to 3 meg in size but fails sometimes on files over that size. However, I have been asked to ensure the form is totally foolproof , i.e. maximum upload size, make sure only an image is uploaded, prevent sql injections. This is where I am struggling. Any help is greatly appreciated.

 

Here is the selection code

 

// Connect to the database
$con = mysql_connect("localhost",$user,$password) or die ('Could not connect: ' . mysql_error());
mysql_select_db($database, $con);
    // Create the form, post to the same file
    echo "<form method='post' action='staffedit.php'>";

    // Form a query to populate the combo-box
    $query = "SELECT id, name FROM staff ORDER BY name;";

    // Successful query?
    if($result = mysql_query($query))  {

      // If there are results returned, prepare combo-box
      if($success = mysql_num_rows($result) > 0) {
        // Start combo-box
        echo "<select name='staffmember'>\n";
        echo "<option>name</option>\n";

        // For each item in the results...
        while ($row = mysql_fetch_array($result))
          // Add a new option to the combo-box
          echo "<option value='$row[name]'>$row[name]</option>\n";

        // End the combo-box
        echo "</select>\n";
      }
      // No results found in the database
      else { echo "No results found."; }
    }
    // Error in the database
    else { echo "Failed to connect to database."; }

    // Add a submit button to the form
    echo "<input type='submit' value='Submit' /></form>";
  
?>

 

This then leads to an image and text upload form, shown below: I've tried to stop files over 3meg from being uploaded but it just prints "You have uploaded a photo of "JOE BLOGGS" and then does nothing else, ideally I'd like it to give a warning and return to the select picture form.

 

<?php	 

$name = $_POST['staffmember'];
echo "<h1> Please Select A Photo of ". $name ."</h1>";
echo "Maximum File Size = 3 Megabytes";
echo "</br>";
echo "Larger Files Take Longer To Upload";
echo"</br>";
echo "Image and Text Must Be Completed Before Pressing Submit";

?>

<form method="post"  action="staffshowedits.php" enctype="multipart/form-data" name="form1">
<input type="hidden" name="var" value="<?php echo $name?>" />
    <p>
    <!-- MAX_FILE_SIZE must precede the file input field -->
    <input type="hidden" name="MAX_FILE_SIZE" value="30000" />

    	<input name="Photo" type="file" id="Photo"><br>

  	</p>
<?php echo    "<h1>Insert some information about ". $name . "</h1>"; 
?>

    <p> <textarea name="information" cols="60" rows="10" type="text" /> </textarea>
    <p>
    	<input type="submit" name="Submit" value="Submit">
         
</p>
</form>

 

The final page, which resizes the image and stores the link and staff members info in the database is below:

 

<?php
$name = $_POST['var'];
$information = $_POST['information'];
stripslashes( mysql_real_escape_string( $information ) );
$user_name =  ($_POST['user_name']); 
echo "<h1> You have uploaded the following photo of ". $name ."</h1>";
?>



<div id="info">
<?php 

    if ($_FILES['Photo'] != "") 
{  
//***************START OF RESIZE IMAGE******************** 

// The file 
$filename = $_FILES['Photo']['tmp_name']; 

// Set a maximum height and width 
$width = 160; 
$height = 120; 

// Get new dimensions 
list($width_orig, $height_orig) = getimagesize($filename); 

if ($width && ($width_orig < $height_orig)) { 
   $width = ($height / $height_orig) * $width_orig; 
} else { 
   $height = ($width / $width_orig) * $height_orig; 
} 

// Resample 
$image_p = imagecreatetruecolor($width, $height); 
$image = imagecreatefromjpeg($filename); 
imagecopyresampled($image_p, $image, 0, 0, 0, 0, $width, $height, $width_orig, $height_orig); 

// Output 
imagejpeg($image_p, $filename, 100); 

//***************END OF RESIZE IMAGE******************** 

//RENAME PHOTO AND MOVE TO PROPER FOLDER 
$randomnumber = rand(1,100000);

$id = "staff";  //This must be the name of the file on the corresponding webpage
     
$extension = strrchr($_FILES['Photo']['name'],'.');  
$extension = strtolower($extension); 

$save_path = '../images/staff/';  //path to original file to be replaced
     
$NewPhotoName = $id . $extension; 

$filename = $save_path . $id . $randomnumber . $extension; 

move_uploaded_file($_FILES['Photo']['tmp_name'],$filename);
$TimeStamp = (date( "His" ));



/*update records according to name */
$db="DATABASE_NAME";
$link = mysql_connect('localhost', 'USERNAME', PASSWORD');
if (! $link)
die(mysql_error());
mysql_select_db($db , $link) or die("Select Error: ".mysql_error());

$update=mysql_query("UPDATE staff SET imagelink='$filename', info='$information' WHERE name='$name'")or die(mysql_error());
mysql_close($link);


/* Show the changes */
$db="DATABASE_NAME";
$link = mysql_connect('localhost', 'USERNAME', 'PASSWORD');
if (! $link)
die(mysql_error());
mysql_select_db($db , $link) or die("Select Error: ".mysql_error());

$classname=mysql_query("SELECT classname FROM staff WHERE name='$name'");
$row = mysql_fetch_array( $classname );

echo "<img src=\"$filename?$TimeStamp\">";
echo "<br>";		
echo "<h1>and the following text to the website</h1>";


echo stripslashes($information);
echo "<br>";
echo "<a href=\"staffselect.php\">Click Here To Amend Another Member Of Staff</a>";
echo "<br>";
echo "<a href=\"../";
echo $row['classname'];
echo "staff.php\" target =\"blank\">Click Here To View The Amended Web Page(Opens in new window)</a>";

mysql_close($link);

} 

?>
</div>

 

As you can see, I have tried to implement some security and restrict the upload size but could do with a few pointers or maybe some critical evaluation. (Not too harsh, I'm only learning  )

 

Cheers

Pete

[Alex]

You should never reply on a hidden field named MAX_FILE_SIZE. You should be checking the filesize in PHP using filesize(). To prevent mysql injections preform mysql_real_escape_string() on all data before passing it to mysql_query().

[petezaman]

Thanks for that Alex. Which of these would these be correct now?

 

$name = mysql_real_escape_string($_POST['var']);

 

or

 

$name = (mysql_real_escape_string($_POST['var']));

 

If its the top one would I be correct in presuming the next three are right.

 

$information = mysql_real_escape_string( $_POST['information']);

stripslashes( mysql_real_escape_string( $information ) );

$user_name = mysql_real_escape_string( ($_POST['user_name']);

 

I'm gathering that they are the only fields which are being passed to mysql in the code I've given so does that mean the code is fine, security wise?

 

I'll have a look at the link you have provided and hopefully will be able to work out what I have to do.

 

Thanks a million.

 

Pete

[Alex]

You'd do it simply like this:

$username = mysql_real_escape_string($_POST['user_name']);

I don't see why you'd want to preform stripslashes on the input as well.. You'd use that when returning the information from the database if magic quotes are on (which isn't a good idea in the first place).

[petezaman]

Thanks again for the speedy reply Alex, I will implement what you have said tomorrow. I need a break from this pc lol. I've been looking at the max file size stuff and I'm getting confused. Maybe a break will cure that.

 

Cheers again

Pete