2

I have the following code for jquery file upload and resize image. In one of my previous posts another user has told that it is a security risk. Here is the code:

<?php
$output_dir = "../images/img_user/";
if(isset($_FILES["file"]) && !empty($_FILES["file"]))
{
    $ret = array();
    $error =$_FILES["file"]["error"];
    if(!is_array($_FILES["file"]["name"])) //single file
    {
        $fileName = $_FILES["file"]["name"];
        move_uploaded_file($_FILES["file"]["tmp_name"],$output_dir.$fileName);
        //img resize
        require 'imgclass.php';
        $resize_image = new Zebra_Image();
        $resize_image->preserve_aspect_ratio = true;
        $resize_image->source_path = $output_dir.$fileName;
        $ext = trim("$fileName");
        $resize_image->target_path = '../images/img_user/'.$ext;
        if (!$resize_image->resize(128, 128, ZEBRA_IMAGE_NOT_BOXED, 1))
        {
            // if there was an error, let's see what the error is about
            switch ($resize_image->error) {
                case 1:
                    $custom_error= array();
                    $custom_error['jquery-upload-file-error']="Image file could not be found!";
                    echo json_encode($custom_error);
                    die();
                case 2:
                    $custom_error= array();
                    $custom_error['jquery-upload-file-error']="Image file is not readable!";
                    echo json_encode($custom_error);
                    die();
                case 3:
                    $custom_error= array();
                    $custom_error['jquery-upload-file-error']="Could not write target file!";
                    echo json_encode($custom_error);
                    die();
                case 4:
                    $custom_error= array();
                    $custom_error['jquery-upload-file-error']="Unsupported image file format!";
                    echo json_encode($custom_error);
                    die();
                case 5:
                    $custom_error= array();
                    $custom_error['jquery-upload-file-error']="Unsupported target file format!";
                    echo json_encode($custom_error);
                    die();
                case 6:
                    $custom_error= array();
                    $custom_error['jquery-upload-file-error']="GD library version does not support target file format!";
                    echo json_encode($custom_error);
                    die();
                case 7:
                    $custom_error= array();
                    $custom_error['jquery-upload-file-error']="GD library is not installed!";
                    echo json_encode($custom_error);
                    die();
            }//end switch
        }//end resize error
        //end resize
        $ret[]= $fileName;
    }
    echo json_encode($ret);
}
?>

I am using the following library:

http://hayageek.com/docs/jquery-upload-file.php

I have created a htaccess file to prevent code execution in upload folder. Folder permission is 755 and file chmod to 640.

Another user Xorifelse commented as: "Also, the security risks with move_uploaded_file($_FILES["file"]["tmp_name"],$output_dir.$‌​fileName); is also from around the year 2000. 17 years out of date of security measures. Yes, you allow us to upload php files to your webserver without any effort. "

If so what is the security issue involved and how can i prevent it ?? I am a newbie programmer.

SmartCoder
  • 413
  • 2
  • 13
Pamela
  • 684
  • 1
  • 7
  • 21
  • 1
    As you are concatenating the **user-defined** `$fileName` with the `$output_dir`, you should sanitize the `$fileName` such that it does not contain directory names, i.e. it should not be allowed to contain `../../some-system-directory/some-system-command`. – Lars Gyrup Brink Nielsen Nov 14 '17 at 06:23
  • 1
    main risk is `FOLDER_PERMISSION`, if you manage this then no serious problem – helpdoc Nov 14 '17 at 06:25
  • Bigger issue lies with the mime type detection. I don't see that here. Hint use `finfo` .. Note this applies to `php 5.3+` – Rotimi Nov 14 '17 at 06:39
  • @ Akintunde Mime type detection is taken care of by jquery upload..only jpg & png are allowed. – Pamela Nov 14 '17 at 06:41
  • Or is it better to use preg_replace('/[^a-zA-Z0-9\-\._]/','', $filename); – Pamela Nov 14 '17 at 06:44
  • @Lars Gyrup Brink Nielsen Thanks for the help. I did the following: $dangerous_characters = array(" ", '"', "'", "&", "/", "\\", "?", "#"); $fileName = str_replace($dangerous_characters, '_', $fileName); – Pamela Nov 14 '17 at 07:16
  • Much better i did $fileName = $unixtime.mt_rand(0,9).'.'.$fileName_extn; to change the file name to unixtime + random value. – Pamela Nov 14 '17 at 07:35
  • If the filename doesn't matter, this seems like a good solution. Escaping file names is not a task you would like to take on by yourself. There's a lot to it, e.g. https://developer.wordpress.org/reference/functions/sanitize_file_name/ – Lars Gyrup Brink Nielsen Nov 14 '17 at 10:00

0 Answers0