7

I have found following code on internet for secure image upload in php.

I want to know it covers all possible way of attacks in image uploading.

define('MAX_SIZE_EXCEDED', 101);
define('UPLOAD_FAILED', 102);
define('NO_UPLOAD', 103);
define('NOT_IMAGE', 104);
define('INVALID_IMAGE', 105);
define('NONEXISTANT_PATH', 106);

class ImgUploader
{
  var $tmp_name;
  var $name;
  var $size;
  var $type;
  var $error;
  var $width_orig;
  var $height_orig;
  var $num_type;
  var $errorCode = 0;
    var $allow_types = array(IMAGETYPE_GIF, IMAGETYPE_JPEG, IMAGETYPE_PNG);

  function __construct($fileArray)
  {
    foreach($fileArray as $key => $value)
    {
      $this->$key = $value;
    }
    if($this->error > 0)
    {
      switch ($this->error)
      {
        case 1: $this->errorCode = MAX_SIZE_EXCEDED; break;
        case 2: $this->errorCode = MAX_SIZE_EXCEDED; break;
        case 3: $this->errorCode = UPLOAD_FAILED; break;
        case 4: $this->errorCode = NO_UPLOAD; break;
      }
    }
    if($this->errorCode == 0)
    {
      $this->secure();
    }
  }

  function secure()
  {
    //$this->num_type = exif_imagetype($this->tmp_name);
    @list($this->width_orig, $this->height_orig, $this->num_type) = getimagesize($this->tmp_name);

    if(filesize($this->tmp_name) > 1024*1024*1024*5) // allows for five megabytes.  Change this number if need be.
    {
      $this->errorCode = MAX_SIZE_EXCEDED;
      return false;
    }

    if (!$this->num_type)
    {
      $this->errorCode = NOT_IMAGE;
        return false;
    }
    if(!in_array($this->num_type, $this->allow_types))
    {
      $this->errorCode = INVALID_IMAGE;
      return false;
    }
  }

  function getError()
  {
    return $this->errorCode;
  }

  function upload_unscaled($folder, $name)
  {
    return $this->upload($folder, $name, "0", "0");
  }

  function upload($folder, $name, $width, $height, $scaleUp = false)
  {
    // $folder is location to be saved
    // $name is name of file, without file extention
    // $width is desired max width
    // $height is desired max height

    if($this->errorCode > 0)
      return false;

    // deal with sizing
    // if image is small enough to not scale, or upload_unscaled() is called, don't scale
    if((!$scaleUp && ($width > $this->width_orig && $height > $this->height_orig)) || ($width === "0" && $height === "0"))
    {
      $width = $this->width_orig;
      $height = $this->height_orig;
    }
    else
    {
      // if height diff is less than width dif, calc height
      if(($this->height_orig - $height) <= ($this->width_orig - $width))
        $height = ($width / $this->width_orig) * $this->height_orig;
      else
        $width = ($height / $this->height_orig) * $this->width_orig;
    }

    // Resample
    switch($this->num_type)
    {
      case IMAGETYPE_GIF: $image_o = imagecreatefromgif($this->tmp_name); $ext = '.gif'; break;
      case IMAGETYPE_JPEG: $image_o = imagecreatefromjpeg($this->tmp_name); $ext = '.jpg'; break;
      case IMAGETYPE_PNG: $image_o = imagecreatefrompng($this->tmp_name); $ext = '.png'; break;
    }

    $filepath = $folder.(substr($folder,-1) != '/' ? '/' : '');
    if(is_dir($_SERVER['DOCUMENT_ROOT'].$filepath))
      $filepath .= $name.$ext;
    else
    {
      $this->errorCode = NONEXISTANT_PATH;
      imagedestroy($image_o);
      return false;
    }

    $image_r = imagecreatetruecolor($width, $height);
    imagecopyresampled($image_r, $image_o, 0, 0, 0, 0, $width, $height, $this->width_orig, $this->height_orig);

    switch($this->num_type)
    {
      case IMAGETYPE_GIF: imagegif($image_r, $_SERVER['DOCUMENT_ROOT'].$filepath); break;
      case IMAGETYPE_JPEG: imagejpeg($image_r, $_SERVER['DOCUMENT_ROOT'].$filepath); break;
      case IMAGETYPE_PNG: imagepng($image_r, $_SERVER['DOCUMENT_ROOT'].$filepath); break;
    }

    imagedestroy($image_o);
    imagedestroy($image_r);

    return '/'.$filepath;
  }
}

I also have a .htaccess file stored in the "images" folder which turns off file scripts, so no one can execute scripts in the photos folder.

<Files ^(*.jpg)>
order deny,allow
deny from all
</Files>
Options -Indexes
Options -ExecCGI 
AddHandler cgi-script .php .php3 .php4 .php5 .phtml .pl .py .jsp .asp .htm .shtml .sh .cgi

does this enough for security reason, or i have to take some other steps to secure my code and website.

j0k
  • 22,600
  • 28
  • 79
  • 90
air
  • 6,136
  • 26
  • 93
  • 125

5 Answers5

7

The first thing I'd say is to check the file types with Apache's mod_mime, otherwise I can send a HTTP request which says I'm a JPEG file! and you simply trust the file extension like Yeap! you have .jpg extension, but in reality I can inject the PHP source code instead of JPEG data, and later I can execute the PHP source code on your server.

The other thing is to always force the Apache to NOT run any of those image types. You can do that with ForceType Directives.

<FilesMatch "(?i)\.jpe?g$">
    ForceType image/jpeg
</FilesMatch>

Edit:

I didn't see the bottom lines. Turning off file scripts will absolutely helps.

Mahdi
  • 9,247
  • 9
  • 53
  • 74
3

Is this Code Secure?

PHP

In short, no, not necessarily! There is one fatal flaw that the code does not cover, but do not despair, this is very easily solvable!

Whilst the script is testing the extension of the file, it is not testing the MIME type of the file, and therefore could potentially open up a whole world of issues.

Apart from this little vulnerability, the code is pretty secure.

Apache

Forging an HTTP request to look like a JPEG is a very easy thing to do, and whilst PHP can handle this, it is sometimes nicer to ensure this vulnerability has been covered by all angles. A simple fix using the MOD_MIME module in Apache can be used to fix this flaw, as explained below.

HTTPS - Possibly Irrelavent

If you are worried about these files being sniffed between the client and the server then an absolutely essential part of your security is to use an SSL certificate on your website. This will encrypt all data between the client and the server.

However, if you are simply worried about the server side of things, this is not really necessary, albeit advised.

Possible Solutions

The finfo() function

The finfo() function will return the MIME type of the file and therefore indicate whether or not it is a JPEG and can be uploaded.

See Here for more details on this function.

Upload Alternative

Personally, I prefer to use the Colin Verot Upload Class. This class is extremely easy to use, covers all potential security issues, has a huge range of extended functionality using the GD library and it is constantly maintained.

Visit Colin Verot's site here to download and start using the class.

Apache MOD_MIME

The Apache MOD_MIME module will force a check of the MIME type of the files being sent to the server.

Have a look here for more info.

Ben Carey
  • 16,540
  • 19
  • 87
  • 169
1

Code seems good and answers above all also nice I just want to add one more point regarding security:

  • you should store the files in a directory that is outside of the document root (not accessible through an http request) and serve them through a script that checks for the proper authorization first.
s-sharma
  • 1,967
  • 1
  • 15
  • 20
0

Since you turned off all interpretation of files on server, only thing that remains is filename. If you generate your own names or filter user-suplied names well, you have nothing to worry about.

Well.. almost nothing. There may be security error in GD library so using malicious image it might be possible to do something bad during resizing. But it is not something you can handle from PHP, so keep server updated.

Josef Kufner
  • 2,851
  • 22
  • 28
-1

If you don't mind using an existing PHP upload system, this jQuery/PHP upload framework is extremely configurable and amazing.

http://blueimp.github.com/jQuery-File-Upload/

colonelclick
  • 2,165
  • 2
  • 24
  • 33
  • From what I can tell that form (used all over the web) asks the user to implement their own security. It looks lovely but any decent developer can use jquery. The main priority should be security always. – Amy Neville Nov 24 '13 at 09:35
  • That framework includes easy to configure file type restrictions, and the pathing is very flexible, allowing you to store files in any restricted directory or even a directory not in the web root. This simplifies most of the work in making a secure file upload. – colonelclick Apr 29 '16 at 19:06