Could you guys tell me if my code is safe? And i want to know how to improve it. You will see a detailed explanation about the code on the comment blocks inside the code.
profileIMGform.php
require_once 'db.php';
if(!empty($_SESSION['user_id'])){
$user_id = $_SESSION['user_id'];
}else{
$user_id = NULL;
}
// here i check if the user is logged in
if(\array_key_exists('user_id', $_SESSION) && $user_id = \filter_var($_SESSION['user_id'], \FILTER_VALIDATE_INT)){
if($_SERVER['REQUEST_METHOD'] == 'POST'){
//i use this condition because i have another code about file upload inside this file
if(!empty($_FILES['uploadedIMG']['size'])){
$finfo = finfo_open(FILEINFO_MIME_TYPE);
$mimeType = finfo_file($finfo, $_FILES["uploadedIMG"]["tmp_name"]);
$allowedMimeTypes = [
'image/jpeg',
'image/jpg',
'image/png'
];
list($width, $height) = getimagesize($_FILES["uploadedIMG"]["tmp_name"]);
$sizeFILE = filesize($_FILES["uploadedIMG"]["tmp_name"]);
//here i check if the uploaded file is a "valid" image, the function "getimagesize()"
// is not 100% trustable, but i added this function here as a "wall"
if(in_array($mimeType, $allowedMimeTypes) && $width != null && $height != null && $sizeFILE <= 768000){
//if the uploaded file is a "valid" image, then it will execute this code on the
//try/catch
try{
$linkN = $_SESSION['linkN']; //user local folder name
$imgPF = $_SESSION['img']; //user defaut avatar img that is set when they
//resgister the account
$capaPF = $_SESSION['capa']; //user defaut wallpaper that is set when they
//resgister the account
$linkNPFFetch = $linkN;
$folderPFFetch = 'user/'.$linkN.'/'; //this variable is used on the glob()
//function bellow
// here i made a SELECT query to get the slug from the username, just in case
//if they changed the username while logged in, this slug is used on the URL
$perfilUpdated = $conn->prepare("SELECT `slug_UN` FROM `users` WHERE `user_id` = :user_id");
$perfilUpdated->bindParam(':user_id', $user_id, PDO::PARAM_INT);
$perfilUpdated->execute();
$rowPFUpdated = $perfilUpdated->fetch(PDO::FETCH_ASSOC);
$SlgUSPFFetch = $rowPFUpdated['slug_UN'];
$ext = explode('.',$_FILES['uploadedIMG']['name']);
$extension = end($ext); //here i get the extension of the uploaded file,
$newname = mt_rand(10000, 10000000); //generate a random name
$folderPFFetchFILE = $folderPFFetch.'avatar/'.$newname.'_'.time().'.'.$extension;
//here i check if exist a file on the user's folder
$imgUserPasta = glob($folderPFFetch.'avatar/*')[0] ?? NULL;
// i don't remember why i made this variable, anyways it's not relevant,
//you can just forget it
$checkPathEdelet = $folderPFFetch.'avatar/'.($_FILES['uploadedIMG']['name']);
$imagickWrite = 'C:/xampp/htdocs/site/user/'.$linkN.'/avatar/'.$newname.'_'.time().'.'.$extension;
if($imgUserPasta != NULL && $imgUserPasta != $checkPathEdelet){
unlink($imgUserPasta);
}
// here is how i'm 99% sure that i will get only valid images, if the uploaded image
//is not valid it won't execute the Imagick code and will print out a error, but
//i'm using a try/catch and this error won't appers
$imgk = new Imagick($_FILES["uploadedIMG"]["tmp_name"]);
$imgk->thumbnailImage(180, 180, true);
$imgk->writeImage($imagickWrite);
$imgUserPastaIMCK = glob($folderPFFetch.'avatar/*')[0] ?? NULL;
if(file_exists($imgUserPastaIMCK)){
$avatar = $siteURL.'/'.$imgUserPastaIMCK;
}else{
$avatar = $siteURL.'/img/avatar/'.$imgPF.'.jpg';
}
echo json_encode(['avatar' => $avatar]);
}catch(Exception $e){
// even if Imagick will only upload valid images, it's recommended to use other
//ways to also validade images
//so i have 3 "walls":
// 1° - i check the mimetype with "finfo_open()",
// 2° - i check if is a "valid" image with "getimagesize()"
// 3° - Imagick will only upload valid images
// if a user falls here on the "catch" block, i'm 100% sure that it's a "malicious"
//user, so i made this query to delete his profile from my database, altrough it
//don't give any protection against attackers, it will make their life a
//bit harder, because they will need to create another account to try uploaded
//malicious files to my server.
// i know that there's ways to forge a user ID, i don't know if it can be possible
//in my project because i don't generate ID when the user loge in, i store the user
//ID on my database when they register the account, anyways i think that this DELETE
//query will be better than nothing.
$deleteProfile = $conn->prepare("DELETE FROM `users` WHERE `user_id` = :user_id");
$deleteProfile->bindParam(':user_id', $user_id, PDO::PARAM_INT);
$deleteProfile->execute();
session_unset();
session_destroy();
echo json_encode(['delete' => 'true']);
}
}
}
}
}
$conn = null;
?>
This is the code that is inside success on Ajax call, it's not related to security, because important things should be made on back-end, but i will put this code here just to you guys see how exactally my code works:
$('#imgUpload').change(function (event) {
...
...
...
success: function (data){
var jsonPF = JSON.parse(data);
if($.trim(jsonPF.delete)){ // if the data returned from
//the profileIMGform.php contains the array 'delete', then
//te page will be reloaded and the user will seen a error page,
//with the mensage: "You need to be logged in to view this page"
$("img.imgPform").css('opacity', '0.4');
$("#loadingIMG").show();
window.location = siteURL;
}else{
$("#loadingIMG").hide();
$("img.imgPform").css('opacity', '1');
$(imgEdit).attr('src', jsonPF.avatar+'?'+ new Date().getTime());
if(window.matchMedia("screen and (max-width: 1172px)").matches) {
$(imgEditMd).attr('src', jsonPF.avatar+'?'+ new Date().getTime());
}else if(window.matchMedia("screen and (min-width: 1173px)").matches) {
$(imgEditMm).attr('src', jsonPF.avatar+'?'+ new Date().getTime());
}
}
}
