Скрипт безопасной загрузки

Question

Я создаю социальную сеть, в которой пользователи могут загружать изображение профиля. Я просто хочу знать, является ли это безопасным способом сделать это. Благодарю.

<?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

Это мое личное мнение, но я бы сказал следующее:

1. Код должен быть отформатирован, я бы лично посмотрел на PSR-12, поскольку этот стандарт должен соблюдаться, когда это возможно.
2. move_uploaded_file не защищает от обхода каталога. Вы должны использовать базовое имя $_FILES['fileToUpload']['tmp_name']и некоторые другие формы проверки
3. Проверка расширения файла с помощью if(in_array($file_ext,$extensions)=== false)не мешает пользователю загружать вредоносный файл, они могут, например, использовать магический байт, чтобы заставить сервер думать, что это файл определенного типа. Вам следует взглянуть на finfo и первый пример загрузки файлов.
4. Вы создаете массив ошибок, который в настоящее время проверяется оператором if и затем отбрасывается. Если вы не планируете использовать его, возможно, лучше просто вернуться из функции пораньше, чем продолжать выполнение.
5. В зависимости от того, насколько уникальным должно быть имя файла, вы можете использовать что-то вроде uniqid(mt_rand(), true)
6. move_uploaded_file заменит файл, если он уже существует, вы можете проверить, существует ли он, прежде чем перезаписывать существующий файл. В зависимости от вашего решения для именования это очень маловероятно, но при высокой нагрузке в течение длительных периодов времени это может происходить чаще, чем вы думаете.
7. Вы используете, UPDATE users SET profile_pic = ? WHERE username = ?я бы предположил, что это значение существует в базе данных, так как пользователь должен войти в систему. Однако, если вы не уверены, существует ли поле или нет (я не видел базу данных), я бы лично использовать: INSERT INTO users (profile_pic, username) VALUES (?,?) ON DUPLICATE KEY UPDATE profile_pic=?, username=?это будет вставлено в таблицу, если строка не существует, но обновит ее, если она есть.
8. Вы установили локальную переменную ширину и высоту и сравниваете их с одним и тем же значением. Полагаю, это было сделано для проверки реальных размеров файла?

Надеюсь, это как-то поможет :)