Безопасная загрузка изображений. Основы

Forever

Новичок
Помогите разобрать тему загрузки изображений.
Что лишнее в скрипте ниже, а что нужно добавить/учитывать Какие ошибки допустил
Или, если там все надо сносить, покажите пожалуйста хороший пример загрузки.


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

P.S: скрытое поле MAX_FILE_SIZE имеет какой-нибудь смысл?
Я гружу огромные файлы, и ожидание загрузки все равно есть.

Так вот
Код:
$max_file_size = 500 *1024; //20 * 1024
    $max_height = 640;
    $max_width = 480;
    $allowed = array ( 'image/jpeg', 'image/png');
    $mime = mime_content_type($_FILES["image"]["tmp_name"]);
    $site_path = "D:/usr/hosts/dota/www1";
    $save_path = '/images/avatars/';
 
         
    if (!isset($_FILES['image']) ) {
        $err['img'] = 'Файл не был выбран.';
    }
    elseif (disk_free_space('/') < $max_file_size) {
        $err['img'] = 'Извините, в данный момент на сайте недоступна функция загрузки фотографий  ';
    }
    elseif ( !is_uploaded_file ($_FILES['image']['tmp_name'])
      /*В документации PHP сказано, что в некоторых случаях getimagesize воспринимает файлы как     изображения, даже если они не являются таковыми. И возвращает массив с бессмысленными значениями. Поэтому проверяю на "адекватность" значения в массиве. (http://php.net/manual/ru/function.getimagesize.php) */
        || ( !list($width, $height, $type, $attr) = getimagesize($_FILES["image"]["tmp_name"]) )
        || ( !preg_match("|^[\d]+$|", $width) )
        || ( !preg_match("|^[\d]+$|", $height) )
        || ( !preg_match("|^[0-9]{1,2}$|", $type)  || $type < 1 || $type >18 )    ){
         
        $err['img'] = 'Неизвестная ошибка при загрузке изображения.';
    }
    elseif( !in_array($mime, $allowed) ) {
        $err['img'] = 'Вы можете загружать только изображения формата JPG и PNG.';
    }
    elseif($_FILES["image"]["size"] > $max_file_size) {
        $err['img'] = 'Слишком большой файл. Максимально допустимый для загрузки размер изображения - 20 кб .';
    }
    elseif($width > $max_width ){
        $err['img'] =  'Ширина изображения не должна превышать '.$max_width.' пикселей';
    }
    elseif ($height > $max_height) {
        $err['img'] =  'Высота изображения не должна превышать '.$max_height.' пикселей';
    }
 
    if (!isset($err) ){
     
        if ($mime === 'image/jpeg'){
            $resource = imageCreateFromJPEG($_FILES['image']['tmp_name']) ;
        }
        elseif ($mime === 'image/png'){
            $resource = imageCreateFromPNG($_FILES['image']['tmp_name']) ;
        }
     
        if (!$resource){
            $err['img'] = 'Неизвестная ошибка при загрузке изображения.';
        }
        else{
          
            $name = 'random_name.jpg';
            $new_image = imageCreateTrueColor($width,$height);
            imageCopy($new_image, $resource, 0, 0, 0, 0, $width, $height);
            imageJPEG($new_image, $site_path.$save_path.$name, 100);
            imageDestroy($new_image);
         
        }
     
    }
 
Последнее редактирование:

AnrDaemon

Продвинутый новичок
Самая первая ошибка - нарушение базовой логики.
Начинаем с
$mime = mime_content_type($_FILES["image"]["tmp_name"]);
и потом внезапно…
if (!isset($_FILES['image']) ) {

Дальше, disk_free_space('/') проверяет свободное место в корне текущего каталога, которого может вообще не быть. Про то, что он может не иметь никакого отношения к свободному месту в каталоге сохранения, дамаю, говорить нет необходимости?
Проверять надо всегда имеено тот путь, куда сохраняешь.

Почему не используются исключения? Лестница из elseif во-первых плохо читается, а во-вторых, надо очень чётко представлять себе логику обработки, чтобы получить точно тот результат, который нужен. Одно неверное условие, и либо ничего не будет работать, либо мусор просыпется в основной код.
 

Фанат

oncle terrible
Команда форума
Начнем с того, что к безопасности этот код не имеет никакого отношения.
В первую очередь надо смотреть на расширение файла, поскольку веб-сервер отличает типы файлов по расширению.
 

Forever

Новичок
Начнем с того, что к безопасности этот код не имеет никакого отношения.
В первую очередь надо смотреть на расширение файла, поскольку веб-сервер отличает типы файлов по расширению.
т.е. Надежнее проверять расширение а не mime тип?
 

Вурдалак

Продвинутый новичок
P.S: скрытое поле MAX_FILE_SIZE имеет какой-нибудь смысл?
Нет. Насколько я знаю, предполагается, что браузер должен это поддерживать, просто чтобы сообщить клиенту до момента загрузки, что файл точно не загрузится.

Начнем с того, что к безопасности этот код не имеет никакого отношения.
В первую очередь надо смотреть на расширение файла, поскольку веб-сервер отличает типы файлов по расширению.
Какая разница какое расширение имеет файл у пользователя, когда ты сохраняешь файл под своим именем?
 

Фанат

oncle terrible
Команда форума
Какая разница какое расширение имеет файл у пользователя, когда ты сохраняешь файл под своим именем?
А, сорри, не увидел.
Ну тогда тем более, с безопасностью загрузки заведомо все нормально.
 

Фанат

oncle terrible
Команда форума
Меня другое интересует. поискал в мануале про "бессмысленные" значения, но ничего такого не нашел.
Откуда дровишки?
Пока что бессмысленным мне кажется этот код с проверкой ответа getimagesize
 

Forever

Новичок
Меня другое интересует. поискал в мануале про "бессмысленные" значения, но ничего такого не нашел.
Откуда дровишки?
Пока что бессмысленным мне кажется этот код с проверкой ответа getimagesize[/QUOTE
]
http://php.net/manual/ru/function.getimagesize.php там предостережение в желтом блоке
 

Forever

Новичок
Меня другое интересует. поискал в мануале про "бессмысленные" значения, но ничего такого не нашел.
Откуда дровишки?
Пока что бессмысленным мне кажется этот код с проверкой ответа getimagesize
а, там ниже сказано, что при ошибочном определении файла как изображения, вместо ширины и высоты вернутся нули. Так что надо проверять, чтобы значения были больше нуля
 

fixxxer

К.О.
Партнер клуба
@Forever, с точки зрения безопасности с аплоадом есть 3 вещи:
1) не допустить ситуации, когда загруженный файл будет выполнен как php-код (ну или код на другом языке). Для этого достаточно автоматически генерировать имена файлов. Допустим, я залью png, где в метаданных будет что-то типа <?php eval($_GET['x'])); ?>. Пока у такого файла не будет расширения php (или иные настройки вебсервера не приведут к интерпретации содержимого этого файла как php-кода), никаких проблем не будет. Да даже если я тупо возьму php-файл, переименую в .png и залью, и даже если ты уберешь все проверки на валидность изображения - при условии, что имя у тебя автогенерируется, будет "битая картинка" и все.
2) эксплоиты для gd/imagemagick, тут все просто - своевременно обновлять серверное ПО
3) DoS-атаки - возможность залить картинку, которая при открытии выжрет кучу ресурсов. Вот тут уже уместно проверять размеры картинки (как в мегабайтах так и в пикселях) прежде чем ее кормить gd/imagick-у.

Все остальное к безопасности отношения не имеет.
 

Forever

Новичок
Самая первая ошибка - нарушение базовой логики.
Точно, лоханулся.

Дальше, disk_free_space('/')
Это я временно, а вообще да, знаю что нужно проверять каталог куда грузишь

Почему не используются исключения?
Пока не дошел до них и до ООП. Но буду применять позже.
 

Вурдалак

Продвинутый новичок
Так то у меня ни в одном браузере не сработало :))
Да им надо вообще это из док выпилить, чтобы не вводить никого в заблуждение, ведь сто процентов кто-то на это значение полагается. Вероятно, у них была надежда навязать какой-то стандарт браузерам, другого объяснения не вижу.
 

fixxxer

К.О.
Партнер клуба
Судя по тому, что вне контекста php оно вообще нигде не встречается, происхождение, полагаю, можно установить, посмотрев авторство первого коммита с этим MAX_FILE_SIZE в php.
 
Сверху