Rate the security of the script

function upload($dir, $validext = array()) { info = ''; //Имя загружаемого файла $file = $_FILES['file']['name']; //Временная папка $tmp = $_FILES['file']['tmp_name']; //Ошибки $error = $_FILES['file']['error']; //Получаем расширения загружаемого файла $ext = pathinfo($file, PATHINFO_EXTENSION); //Новое имя файла $name = time().'_'.rand(0,10000000).'.'.$ext; if($error === UPLOAD_ERR_OK) { switch($ext) { case 'jpg' : $im = imagecreatefromjpeg($tmp); break; case 'jpeg': $im = imagecreatefromjpeg($tmp); break; case 'gif' : $im = imagecreatefromgif($tmp); break; case 'png' : $im = imagecreatefrompng($tmp); break; } //Смотрим, картинка ли загружается if(!$im) $info = '<p>Не картинка</p>'; else { //Создаем папку для перемещения изображений, если таковой нет if(!is_dir($dir)) mkdir($dir, 0777); //Валидация расширения if(!in_array($ext, $validext)) $info = 'Не тот формат'; else { //Перемещаем файл в нужную директорию if(move_uploaded_file($tmp, $dir.$name)) $info = 'Файл загружен'; else $info = 'Ошибка зарузки файла'; } } } else { //Формируем ошибки switch($error) { case 1: $info = '<p>Файл слишком большой</p>'; break; case 2: $info = '<p>Файл слишком большой</p>'; break; case 3: $info = '<p>Файл Загружен частично</p>'; break; case 4: $info = '<p>Вы не выбрали файл для загрузки</p>'; break; case 5: $info = '<p>Вы не выбрали файл для загрузки</p>'; break; case 6: $info = '<p>Не найдена папка для временных файлов</p>'; break; default: $info = '<p>Ошибка записи файла на диск</p>'; break; } } return $info; }

    2 answers 2

    I would assign the $ im variable to null before using it. Well, I would endorse the format validation to the beginning (right now it doesn’t carry much sense). And so ok.

    • Got it! Thank you - vinnie
    • check the file format and get the extension, better through mime types, as experience shows, users love to change the file extension for example from .bmp to .jpg.
    • switch constructions are not needed here at all. readability of the code at times falls; for errors, it is better to immediately create an array or a class with a code and an error message, then using the array key with errors as a code to issue the necessary message.
    • Before writing to the directory, you need to check the access rights, ideally, to give an error about the impossibility of writing to this directory, not all php scripts are executed from the root.
    • for the $ dir variable, it would be nice to also check for existence from the root, depending on the php settings, is_dir ('/ images /') can return false even if the directory actually exists.