Script de upload seguro

Question

Estou criando uma rede social que permite aos usuários carregar uma foto de perfil. Só quero saber se essa é uma maneira segura de fazer isso. Obrigado.

<?php

include 'includes/header.php';
include 'includes/form_handlers/settings_handler.php';

//$userPic = ''; $date_time = date('Y-m-d_H-i-s');

if(!empty($userLoggedIn)) { if (isset($_FILES['fileToUpload'])) {
        $errors= array(); $file_name = $_FILES['fileToUpload']['name']; $file_size = $_FILES['fileToUpload']['size']; $width = 1500;
        $height = 1500; $file_tmp = $_FILES['fileToUpload']['tmp_name']; $file_type = $_FILES['fileToUpload']['type']; $tmp = explode('.',$_FILES['fileToUpload']['name']); $file_ext=strtolower (end ($tmp)); $extensions = array( "jpeg", "jpg", "png", "gif");

        if(in_array($file_ext,$extensions)=== false){

            $errors[]="extension not allowed, please choose a JPEG or PNG file."; } if ($file_size > 8097152) {

            $errors[] = 'File size must be 2 MB'; } if ($width > 1500 || $height > 1500) { echo"File is to large"; } if(!$errors) {

            $userPic = md5($_FILES["fileToUpload"]["name"]) . $date_time . " " . $file_name;
            $profilePic = move_uploaded_file($file_tmp,"assets/images/profile_pics/" . $userPic); $file_path = "assets/images/profile_pics/" . $userPic; $stmt = $con->prepare("UPDATE users SET profile_pic = ? WHERE username = ?"); $stmt->bind_param('ss', $file_path, $username);
            $stmt->execute(); $stmt->close();

            header('Location: settings.php');
            exit();
 
        }
    }
} else {

    echo "Invalid Username";
}

?>

Answer 1

Esta é minha opinião pessoal, mas diria o seguinte:

1. O código deve ser formatado, eu pessoalmente consideraria o PSR-12, pois esse padrão deve ser seguido sempre que possível.
2. move_uploaded_file não protege contra passagem de diretório. Você deve usar o nome de base em $_FILES['fileToUpload']['tmp_name']e algumas outras formas de validação
3. Verificar a extensão do arquivo com if(in_array($file_ext,$extensions)=== false)não impede que um usuário envie um arquivo malicioso, ele poderia, por exemplo, usar um byte mágico para enganar o servidor e fazê-lo pensar que é um determinado tipo de arquivo. Você deve dar uma olhada no finfo e no primeiro exemplo de upload de arquivo
4. Você está criando uma série de erros, que estão sendo verificados em uma instrução if e, em seguida, descartados. Se você não está planejando usá-lo, pode ser melhor apenas retornar da função mais cedo do que continuar a execução.
5. Dependendo de quão único o nome do arquivo deve ser, você pode querer usar algo como uniqid(mt_rand(), true)
6. move_uploaded_file substituirá um arquivo se ele já existir, você pode querer verificar se ele existe antes de sobrescrever um arquivo existente. Dependendo da sua solução de nomenclatura, é muito improvável que ocorra, mas sob alta carga por longos períodos de tempo isso pode acontecer com mais frequência do que você pensa.
7. Você está usando UPDATE users SET profile_pic = ? WHERE username = ?, suponho que esse valor existe no banco de dados porque o usuário precisa estar logado. No entanto, se você não tiver certeza se o campo existe ou não (não vi o banco de dados), usar pessoalmente: INSERT INTO users (profile_pic, username) VALUES (?,?) ON DUPLICATE KEY UPDATE profile_pic=?, username=?será inserido na tabela se a linha não existir, mas será atualizado se existir.
8. Você definiu uma variável local chamada largura e altura e está comparando-as com o mesmo valor. Suponho que isso foi feito para verificar as dimensões reais do arquivo.

Espero que isso ajude de algum jeito :)