how to secure this image upload code

Question

I have been hacked during last two weeks and I see in the log files that hacker can upload php shells in registration

this is the form which let user register and save the data to database in the site and the point is when he uploads his avatar he can upload php file

<form method="post" name="regiterationForm" id="regiterationForm" enctype="multipart/form-data">
<table width="930" border="0" align="center" cellpadding="2" cellspacing="2">

<tr>
<td width="431" valign="top"><table width="500" border="0" cellspacing="2" cellpadding="2">
<tr>
<td width="215" align="right" class="registerpage_form_credential">Username:</td>
<td colspan="3" class="registersignupbox_bg">
<input onBlur="checkUserName(this.value);" type="text" name="userName" id="userName" class="register_signup_field" /> </td>
<div class="errormsg" id="error_userName" style="display:none;"><span class="noberr"></span><span id="error_userName_val">Please enter name</div>
</tr>
<tr>
<td width="215" align="right" class="registerpage_form_credential">Password:</td>
<td colspan="3" class="registersignupbox_bg"><input type="password" name="userPassword" id="userPassword" class="register_signup_field" />&nbsp;</td>
<div class="errormsg errpass" id="error_userPassword" style="display:none;"><span class="noberr"></span><span id="error_userPassword_val">Please enter password</div>
</tr>
<tr>
<td width="215" align="right" class="registerpage_form_credential">Retype Password:</td>
<td colspan="3" class="registersignupbox_bg"><input type="password" name="userRetypePassword" id="userRetypePassword" class="register_signup_field" />&nbsp;</td>
<div class="errormsg errpass1" id="error_userRetypePassword" style="display:none;"><span class="noberr"></span><span id="error_userRetypePassword_val">Please enter password</div>
</tr>
<tr>
<td width="215" align="right" class="registerpage_form_credential">Your real name:</td>
<td colspan="3" class="registersignupbox_bg"><input type="text" name="userRealName" id="userRealName" class="register_signup_field" />&nbsp;</td>
<div class="errormsg errpass2" id="error_userRealName" style="display:none;"><span class="noberr"></span><span id="error_userRealName_val">Please enter Name</div>
</tr>
<tr>
<td align="right" class="registerpage_form_credential">E-mail address:</td>
<td colspan="3" class="registersignupbox_bg"><input onBlur="checkUserEmail(this.value);" type="text" name="userEmail" id="userEmail" class="register_signup_field" />
&nbsp;</td>
<div class="errormsg errpass3" id="error_userEmail" style="display:none;"><span class="noberr"></span><span id="error_userEmail_val">Please enter Email</div>
</tr>
<tr>
<td align="right" class="registerpage_form_credential">Retype E-mail address:</td>
<td colspan="3" class="registersignupbox_bg"><input type="text" name="userRetypeEmail" id="userRetypeEmail" class="register_signup_field" />
&nbsp;</td>
<div class="errormsg errpass4" id="error_userRetypeEmail" style="display:none;"><span class="noberr"></span><span id="error_userRetypeEmail_val">Please enter Email</div>
</tr>
<tr>
<td align="right" class="registerpage_form_credential">Country:</td>
<td colspan="3" class="registersignupbox_bg"><select name="country" id="country" class="register_monthday_selection">
<option value="">Choose a Country</option>
<?php for($i=0; $i<count($allCountryName); $i++){ ?>
    <option value="<?php echo $allCountryName[$i]['name']; ?>"><?php echo $allCountryName[$i]['name']; ?></option>
<?php } ?>
</select>
&nbsp;</td>
<div class="errormsg errpass5" id="error_country" style="display:none;"><span class="noberr"></span><span id="error_country_val">Please Select Country</div>
</tr>
<tr>
<td align="right" class="registerpage_form_credential">Date of birth:</td>

<td class="register_day_bg"><select name="userDay" id="userDay" class="register_day_selection">
<option value="">Day</option>
<?php for($i=1; $i<=31; $i++){ ?>
    <option value="<?php echo $i; ?>"><?php echo $i; ?></option>
<?php } ?>
</select></td>

<td class="register_month_bg"><select name="userMonth" id="userMonth" class="register_month_selection">
<option value="">Month</option>
<?php $month_digit_array = array("1"=>"January","2"=>"February","3"=>"March","4"=>"April","5"=>"May","6"=>"June","7"=>"July","8"=>"August","9"=>"September","10"=>"October","11"=>"November","12"=>"December"); 
    for($i=1; $i<=12; $i++){
?>
<option value="<?php echo $i; ?>"><?php echo $month_digit_array[$i]; ?></option>
<?php } ?>
</select></td>

<td class="register_year_bg"><select name="userYear" id="userYear" class="register_year_selection">
<option value="">Year</option>
<?php $curYear = date('Y');  for($i=$curYear; $i>=1930; $i--){ ?>
    <option value="<?php echo $i; ?>"><?php echo $i; ?></option>
<?php } ?>
</select></td>
</tr>

<tr>
<td align="right" class="registerpage_form_credential">I am a:</td>
<td colspan="3" class="registersignupbox_bg"><select name="gender" id="gender" class="register_monthday_selection">
<option value="">Select an option</option>
<option value="Male">Male</option>
<option value="Female">Female</option>

</select></td>
<div class="errormsg errpass6" id="error_gender" style="display:none;"><span class="noberr"></span><span id="error_gender_val">Please Select Gender</div>
</tr>
<tr>
<td align="right" class="registerpage_form_credential">Select an Avatar:</td>
<td colspan="3" class="registersignupbox_bg"><input type="file" id="image" name="userAvatar" /></td>
<div class="errormsg errpass7" id="error_image" style="display:none;"><span class="noberr"></span><span id="error_image_val">Please Select Image</div>
</tr>
<tr>
<td colspan="4" style="
    padding-left: 37%;
"><div class="g-recaptcha" data-sitekey="6LfhvAkTAAAAAFQyqmN2nf9hXEY1T0jF89SCGNVB"></div>
</td>
</tr>
</table></td>
<td width="455">

<p>

<iframe width="555" height="312" src="https://www.youtube.com/embed/vkjFm6ClTuw" frameborder="0" allowfullscreen></iframe>

</p>

<br />
<div style="width:468px ;margin: 0 auto; height:60px">

<script async src="//pagead2.googlesyndication.com/pagead/js/adsbygoogle.js"></script>
<!-- Header/Footer Small -->
<ins class="adsbygoogle"
     style="display:inline-block;width:468px;height:60px"
     data-ad-client="ca-pub-5794587985510139"
     data-ad-slot="5064971004"></ins>
<script>
(adsbygoogle = window.adsbygoogle || []).push({});
</script>

</div>

</td>
</tr>
<tr>
<td colspan="2"><table width="709" border="0" align="center" cellpadding="2" cellspacing="2" style="margin-top:11px;    margin-top: 15px !important;
margin-right: 67px;" class="signp">
<tr>
<td>&nbsp;</td>
<td class="contactus_form_credentials">&nbsp;</td>
</tr>
<tr>
<td width="21" align="center"><label>
<input type="checkbox" name="promotion" id="promotion"/>
</label></td>
<td width="442" class="contactus_form_credentials">I agree that my account will be deleted if my email is not valid</td>
</tr>
<tr>
<td align="center"><input type="checkbox" name="checkbox2" id="checkbox2"/></td>
<td class="contactus_form_credentials">I have read and agree to the (example.com) <a href="terms.php" class="termsofuse">Terms of use</a></td>
</tr>
<tr>
<td>&nbsp;</td>
<td style="cursor:pointer;" onclick="registerFormValidation();"><img src="images/register_signup_btn.png" width="220" height="108" /></td>
</tr>
</table></td>
</tr>
</table>




<div class="errormsg errpass7" id="error_captcha" style="display:none;"><span class="noberr"></span><span id="error_captcha_val"></div>






</form>

and this the function for registration

function insertData($arr,$files){
    $username  =$arr['userName'];
    $pass      =$arr['userPassword'];
    $realname  =$arr['userRealName'];
    $email     =$arr['userEmail'];
    $country   =$arr['country'];
    $gender    =$arr['gender'];
    $dob       =$arr['userDay'].'-'.$arr['userMonth'].'-'.$arr['userYear'];
    if(self::userNameExit($username) != "not_exist"){
        echo "<script>document.location.href='".HTTP_PATH."register.php'</script>";
        exit();
    }
    if(self::userEmailExist($email) != "not_exist"){
        echo "<script>document.location.href='".HTTP_PATH."register.php'</script>";
        exit();
    }
    require_once(COMM_PATH."DatabaseManager.php"); 
    $db= new DatabaseManager();
    $emailcode =  rand().time();
    $sql       =  "Insert into users(username,pass,realname,email,country,dob,gender,lastlogin,register_date,email_varification,email_code)values('".$username."','".md5($pass)."','".$realname."','".$email."','".$country."','".$dob."','".$gender."', now(), now(),'n','".$emailcode."')";
    $result    =  $db->executeUpdate($sql);
    $user_id   =  $db->lastInsertId();
    self::autoFriends($user_id);
    self::sentMessage($user_id,$username);
    insertStatusNow($user_id,"user");
    if(isset($files['userAvatar']['name']) && $files['userAvatar']['name'] !=""){
        $nameOfImage    = time()."_".basename($files['userAvatar']['name']);
        $path           = USER_IMAGE_UPLOAD_PATH.$nameOfImage;
        if(move_uploaded_file($files['userAvatar']['tmp_name'], $path)){
            $sql="UPDATE `users` SET `image` = '".$nameOfImage."' WHERE `Id` =".$user_id;
            $db->executeUpdate($sql);
            if(isset($arr['promotion']) && $arr['promotion'] == "promotion_yes"){
                self::notificationSubmit($user_id,$email);
            }
        }
        return $user_id;
    }
}

I want to know what is wrong with me , the business is going down every day the site hacked

UPDATE ::

    if(isset($files['userAvatar']['name']) && $files['userAvatar']['name'] !=""){
            $nameOfImage    = $files['userAvatar']['name'];
            $path           = USER_IMAGE_UPLOAD_PATH.$nameOfImage;


$fileInfo = new finfo();

$allowedMIMETypes = ['image/bmp', 'image/gif', 'image/jpeg', 'image/png']; //the most common image MIME types, you can add more if you want

if(in_array($fileType = $fileInfo->file($nameOfImage, FILEINFO_MIME_TYPE, $allowedMIMETypes))){
                if(move_uploaded_file($files['userAvatar']['tmp_name'], $path)){
                $sql="UPDATE `users` SET `image` = '".$nameOfImage."' WHERE `Id` =".$user_id;
                $db->executeUpdate($sql);
                if(isset($arr['promotion']) && $arr['promotion'] == "promotion_yes"){
                    self::notificationSubmit($user_id,$email);
                }
            }
            return $user_id;
} 

else {
    die("check you reg");
}
return null;

I have made the code like that but it let user register but with no photo

Where are you stripping any HTML content before inserting it into your Database? Use PDO prepare statements: it isn't depreciated in PHP 7. Anyone can simply put their username as, for example: `` — Jaquarh, Apr 11 '16 at 11:32
Possible duplicate of [How can I prevent SQL-injection in PHP?](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) — Jaquarh, Apr 11 '16 at 11:34
@KyleE4K its not sql injection, it is file upload vulnerability. Suppose an attacker uploads c99.php . Then when he goes to that path, the server will run the php script thus giving him a shell access. More can be found here http://www.r57c99.com/ — georoot, Apr 11 '16 at 11:38
Still, I see no sanitizing of inputs in your code. Also, why not restrict file extensions (create an object) which searches file extension names and uses a callback method to switch if its acceptable or not? — Jaquarh, Apr 11 '16 at 11:40

Jonan · Accepted Answer · 2016-04-11T13:28:47.683

Use this to check the MIME-type of the file:

$file = $files['userAvatar']['name'];
$fileInfo = new finfo();

$allowedMIMETypes = ['image/bmp', 'image/gif', 'image/jpeg', 'image/png']; //the most common image MIME types, you can add more if you want

if(in_array($fileType = $fileInfo->file($file, FILEINFO_MIME_TYPE, $allowedMIMETypes)){
    //the file is an image
} else {
    //file doesn't have a whitelisted MIME-type
}

Update: If you want to prevent users from registering without an avatar, simply put the code I gave you before you insert the user into the database. Your entire function would become something like this:

function insertData($arr,$files){
    $username  =$arr['userName'];
    $pass      =$arr['userPassword'];
    $realname  =$arr['userRealName'];
    $email     =$arr['userEmail'];
    $country   =$arr['country'];
    $gender    =$arr['gender'];
    $dob       =$arr['userDay'].'-'.$arr['userMonth'].'-'.$arr['userYear'];
    if(self::userNameExit($username) != "not_exist"){
        echo "<script>document.location.href='".HTTP_PATH."register.php'</script>";
        exit();
    }
    if(self::userEmailExist($email) != "not_exist"){
        echo "<script>document.location.href='".HTTP_PATH."register.php'</script>";
        exit();
    }

    //*************
    if(!isset($files['userAvatar']['name']) || empty($files['userAvatar']['name'])){
        //no file was uploaded, give some kind of error message
    }

    $fileInfo = new finfo();
    $allowedMIMETypes = ['image/bmp', 'image/gif', 'image/jpeg', 'image/png']; //the most common image MIME types, you can add more if you want
    if(!in_array($fileType = $fileInfo->file($nameOfImage, FILEINFO_MIME_TYPE, $allowedMIMETypes))){
        //the uploaded file wasn't recognized as an image, give some kind of error message
    }
    *************//

    require_once(COMM_PATH."DatabaseManager.php"); 
    $db= new DatabaseManager();
    $emailcode =  rand().time();
    $sql       =  "Insert into users(username,pass,realname,email,country,dob,gender,lastlogin,register_date,email_varification,email_code)values('".$username."','".md5($pass)."','".$realname."','".$email."','".$country."','".$dob."','".$gender."', now(), now(),'n','".$emailcode."')";
    $result    =  $db->executeUpdate($sql);
    $user_id   =  $db->lastInsertId();
    self::autoFriends($user_id);
    self::sentMessage($user_id,$username);
    insertStatusNow($user_id,"user");
    $nameOfImage    = time()."_".basename($files['userAvatar']['name']);
    $path           = USER_IMAGE_UPLOAD_PATH.$nameOfImage;
    if(move_uploaded_file($files['userAvatar']['tmp_name'], $path)){
        $sql="UPDATE `users` SET `image` = '".$nameOfImage."' WHERE `Id` =".$user_id;
        $db->executeUpdate($sql);
        if(isset($arr['promotion']) && $arr['promotion'] == "promotion_yes"){
            self::notificationSubmit($user_id,$email);
        }
    }
    return $user_id;
}

Furthermore, I saw that you handled errors by redirecting the user to the registration page using JavaScrip (not even using the PHP header() function). This isn't very elegant, and the user won't know what they did wrong, so I suggest you return an error message from the function instead, which is then echoed on the registration page

great it worked but I am sorry If i ask more do you have an idea to prevent uploading files like that image.php.jpg ? thank you — Petter Adam, Apr 11 '16 at 14:06
@PetterAdam This code doesn't check for file extension but for the MIME-type, so even if hackers would upload a php file called image.php.jpg, it won't be recognized as a jpg image and an error message will fire — Jonan, Apr 11 '16 at 14:08
@PetterAdam The image upload part is. However, as Kyle E4K commented, your code is still very prone to SQL-injection. This can be resolved using prepared statements - read more about that here: http://stackoverflow.com/questions/60174/ — Jonan, Apr 11 '16 at 14:19

score 0 · Answer 2 · answered Apr 11 '16 at 11:36

So what you want is that people only upload images and not PHP script. Well the answer to that is simple, check the file extention. But that being said there are methods like injecting %00(Not sure about the opcode) to fool the PHP server. It is better to get an existing library that checks that for you. Once that is done, in order that the uploaded script is not executes, don't give it executable right. That way even if an attacker uploads php file, It won't be processed by your server. And if you check the extention properly, When the attacker suppose uploads c99 shell, will only see a broken image in upload path and not the actual shell page, because it is being though of an image by the web server

how to secure this image upload code

2 Answers2