Verifying image uploads are really images!

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

Post Reply
Javrixx
Forum Commoner
Posts: 32
Joined: Thu Aug 24, 2006 2:05 pm

Verifying image uploads are really images!

Post by Javrixx »

Everah | Please use

Code: Select all

,

Code: Select all

and [syntax="..."] tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read:  [url=http://forums.devnetwork.net/viewtopic.php?t=21171]Posting Code in the Forums[/url] to learn how to do it too.[/color]


I've been searching for 3 days and can't find the answer.  Either the code I put in doesn't work or I'm a moron.  I am pretty new to PHP, so that could be a major issue.  What I have setup is a page where people can upload images.  This works just fine, the script as of right now works perfect, no problems.

But after reading around, I found people can just rename a .php or some other file to just a .jpg or .gif, upload it, and then execute it...  I do not want this to happen!

So the solution everyone is talking about is using one of two codes to verify the file is actually an image that is being uploaded.

getimagesize() and exif_imagetype() are the two codes. I can't get either to work. I'm just learning PHP for the first time, so I'm very unfamiliar with it. What I need to know is... what code do I use and where do I put it in my script so it will verify the file is really an image. I don't care which one is used. The exif is supposed to be faster, but I don't really care.

Below is my script. Thanks!

Code: Select all

<?
$num_of_uploads=1;
$file_types_array=array("jpg","gif");
$max_file_size=110000;
$upload_dir="images_public/";
function uploaderFILES($num_of_uploads=1, $file_types_array=array("jpg","gif"), $max_file_size=110000, $upload_dir="images_public/"){
  if(!is_numeric($max_file_size)){
   $max_file_size = 110000;
  }  
  foreach($_FILES["file"]["error"] as $key => $value)
  {
     if($_FILES["file"]["name"][$key]!="")
     {
       if($value==UPLOAD_ERR_OK)
       {
         $origfilename = $_FILES["file"]["name"][$key];
         $filename = explode(".", $_FILES["file"]["name"][$key]);
         $filenameext = $filename[count($filename)-1];
         unset($filename[count($filename)-1]);
         $filename = implode(".", $filename);
         $filename = substr($filename, 0, 15).".".$filenameext;
         $file_ext_allow = FALSE;
if (file_exists('images_public/' . $filename)) {
  $tmpVar = 1;
  while(file_exists('images_public/' . $tmpVar . '-' . $filename)) {
   $tmpVar++;
   }
  $filename= $tmpVar . '-' . $filename;
  }        
         for($x=0;$x<count($file_types_array);$x++){
           if($filenameext==$file_types_array[$x])
           {
             $file_ext_allow = TRUE;
           }
         }
         if($file_ext_allow){
           if($_FILES["file"]["size"][$key]<$max_file_size){
             if(move_uploaded_file($_FILES["file"]["tmp_name"][$key], $upload_dir.$filename)){
               echo("<center>File uploaded successfully. Your image can be found at <a href='http://www.averageguysteve.com/".$upload_dir.$filename."' target='_blank'>http://www.averageguysteve.com/".$upload_dir.$filename."</a><br /><br /><br /><img src='".$upload_dir.$filename."' border='0' alt=''></center>");
             }
             else { echo('<center><font color="#FF0000">'.$origfilename."</font> was not successfully uploaded.<br /></center>");}
           }
           else  { echo('<center><font color="#FF0000">'.$origfilename."</font> was too big and was not uploaded. Max file size is 100k!<br /></center>"); }
         }
         else{ echo('<center><font color="#FF0000">'.$origfilename." </font>had an invalid file extension and was not uploaded. Valid file types are .jpg or .gif.<br /></center>");  }
       }
       else{ echo('<center><font color="#FF0000">'.$origfilename." </font>was not successfully uploaded.<br /></center>");  } // else
     }
  }
}

?>

<HTML>
<BODY>

  <FORM action='<?=$PHP_SELF;?>' method='post' enctype='multipart/form-data'>Upload file:<BR /><INPUT type='hidden' name='submitted' value='TRUE' id='<?=time();?>' >
  <INPUT type='hidden' name='MAX_FILE_SIZE' value='<?=$max_file_size;?>' >
<?  for($x=0;$x<$num_of_uploads;$x++){
     $form .= "<input type='file' name='file[]'><br />";
   }
   $form .= "<input type='submit' value='Upload'><br /><br />
   <font color='red'>*</font>Max file size is 100k.  Valid file types are .";
   for($x=0;$x<count($file_types_array);$x++){
     if($x<count($file_types_array)-1){
       $form .= $file_types_array[$x]." or .";
     }else{
       $form .= $file_types_array[$x].".";
     }
   }
   echo($form);
?>  
  </FORM>
</BODY>
</HTML>

<?
if(isset($_POST["submitted"])){
   uploaderFILES($num_of_uploads, $file_types_array, $max_file_size, $upload_dir);
}
?>

Everah | Please use

Code: Select all

,

Code: Select all

and [syntax="..."] tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read:  [url=http://forums.devnetwork.net/viewtopic.php?t=21171]Posting Code in the Forums[/url] to learn how to do it too.[/color]
User avatar
volka
DevNet Evangelist
Posts: 8391
Joined: Tue May 07, 2002 9:48 am
Location: Berlin, ger

Post by volka »

viewtopic.php?t=54395 may be of interest.
User avatar
pickle
Briney Mod
Posts: 6445
Joined: Mon Jan 19, 2004 6:11 pm
Location: 53.01N x 112.48W
Contact:

Post by pickle »

PHP Manual wrote:If accessing the filename image is impossible, or if it isn't a valid picture, getimagesize() will return FALSE and generate an error of level E_WARNING.
I'd guess you'd put it in your code here:

Code: Select all

...
if($filenameext==$file_types_array[$x] || !@getimagesize($filename))
{
    $file_ext_allow = TRUE;
}
...
FYI: The '@' will suppress the possible warning getimagesize() may generate.
Real programmers don't comment their code. If it was hard to write, it should be hard to understand.
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

Avoid the "@" operator.

Code: Select all

$err = error_reporting(0);
$info = getimagesize($filename);
error_reporting($err);
User avatar
William
Forum Contributor
Posts: 332
Joined: Sat Oct 25, 2003 4:03 am
Location: New York City

Post by William »

feyd wrote:Avoid the "@" operator.

Code: Select all

$err = error_reporting(0);
$info = getimagesize($filename);
error_reporting($err);
[at] feyd
I was just wondering what the reasoning for avoiding the "@" operator is?
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

It's slower than the posted snippet.
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post by RobertGonzalez »

Not to mention that if there is a potential for the error, you can code error trapping/checking mechanisms into the code to handle this, instead of just quieting them. This makes for more extensible, potable, efficient code.
Javrixx
Forum Commoner
Posts: 32
Joined: Thu Aug 24, 2006 2:05 pm

Post by Javrixx »

Sorry guys, I don't know if I'm just retarded or what, but both of what you posted below doesn't seem to work. I can still upload non-images that just have a .jpg extension... Again, I'm really new at this so please be a little more basic with me as far as the code goes.
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

What's the new code?
Javrixx
Forum Commoner
Posts: 32
Joined: Thu Aug 24, 2006 2:05 pm

Post by Javrixx »

Actually I got it fixed. I guess my host doesn't support the exif command because I just get a fatal error when using it, but the imagesize code works fine. Here is the new code, only one line is altered really and just added the extra closing bracket. But now my error messages aren't showing up correctly, I'm going to stare at the code and try to soak it up now, see why it doesn't display propery.

The actually page is http://www.averageguysteve.com/upload.php



Code: Select all

<?
$num_of_uploads=1;
$file_types_array=array("jpg","gif");
$max_file_size=110000;
$upload_dir="images_public/";
function uploaderFILES($num_of_uploads=1, $file_types_array=array("jpg","gif"), $max_file_size=110000, $upload_dir="images_public/"){
  if(!is_numeric($max_file_size)){
   $max_file_size = 110000;
  } 
  foreach($_FILES["file"]["error"] as $key => $value)
  {
     if($_FILES["file"]["name"][$key]!="")
     {
       if($value==UPLOAD_ERR_OK)
       {
         $origfilename = $_FILES["file"]["name"][$key];
         $filename = explode(".", $_FILES["file"]["name"][$key]);
         $filenameext = $filename[count($filename)-1];
         unset($filename[count($filename)-1]);
         $filename = implode(".", $filename);
         $filename = substr($filename, 0, 15).".".$filenameext;
         $file_ext_allow = FALSE;
if (file_exists('images_public/' . $filename)) {
  $tmpVar = 1;
  while(file_exists('images_public/' . $tmpVar . '-' . $filename)) {
   $tmpVar++;
   }
  $filename= $tmpVar . '-' . $filename;
  }       
         for($x=0;$x<count($file_types_array);$x++){
           if($filenameext==$file_types_array[$x]) 
           {
             $file_ext_allow = TRUE;
           }
         }
         if($file_ext_allow){
           if($_FILES["file"]["size"][$key]<$max_file_size){
if(getimagesize($_FILES["file"]["tmp_name"][$key])){
             if(move_uploaded_file($_FILES["file"]["tmp_name"][$key], $upload_dir.$filename)){
			 
			 
			 
			 
			 
               echo("<center>File uploaded successfully. Your image can be found at <a href='http://www.averageguysteve.com/".$upload_dir.$filename."' target='_blank'>http://www.averageguysteve.com/".$upload_dir.$filename."</a><br /><br /><br /><img src='".$upload_dir.$filename."' border='0' alt=''></center>");
             }
             else { echo('<center><font color="#FF0000">'.$origfilename."</font> was not successfully uploaded.<br /></center>");}
           }
           else  { echo('<center><font color="#FF0000">'.$origfilename."</font> was too big and was not uploaded. Max file size is 100k!<br /><br /><b>OR</b><br /><br />Your file was not a real image file.  Only images are allowed!<br /></center>"); }
         }
         else{ echo('<center><font color="#FF0000">'.$origfilename." </font>had an invalid file extension and was not uploaded. Valid file types are .jpg or .gif.<br /></center>");  }
       }
       else{ echo('<center><font color="#FF0000">'.$origfilename." </font>was not successfully uploaded.<br /></center>");  } // else
     }
  }
}
}

?>

<HTML>
<TITLE>AverageGuySteve.com - Upload an Image</TITLE>
<BODY>
  <FORM action='<?=$PHP_SELF;?>' method='post' enctype='multipart/form-data'>Upload file:<BR /><INPUT type='hidden' name='submitted' value='TRUE' id='<?=time();?>' >
  <INPUT type='hidden' name='MAX_FILE_SIZE' value='<?=$max_file_size;?>' >
<?  for($x=0;$x<$num_of_uploads;$x++){
     $form .= "<input type='file' name='file[]'><br />";
   }
   $form .= "<input type='submit' value='Upload'><br /><br />
   <font color='red'>*</font>Max file size is 100k.  Valid file types are .";
   for($x=0;$x<count($file_types_array);$x++){
     if($x<count($file_types_array)-1){
       $form .= $file_types_array[$x]." or .";
     }else{
       $form .= $file_types_array[$x].".";
     }
   }
   echo($form);
?> 
  </FORM>
</BODY>
</HTML>

<?
if(isset($_POST["submitted"])){
   uploaderFILES($num_of_uploads, $file_types_array, $max_file_size, $upload_dir);
}
?>
Thanks again for your help guys. Any other suggestions or comments is greatly appreciated as I'm so new to this php thing.
Post Reply