Безопасен ли такой способ загрузки картинок на PHP?

Функция писалась для Yii (да, я знаю о существовании расширений и о наличии валидатора yii, вопрос в безопасности конкретно этой функции, т.к. планируется её расширение).

а) Можно ли упростить код?
б) Достаточно ли он безопасен?
в) Предположим, хакер умудрился дать серверу возможность исполнять .jpg, .gif и .png файлы. Есть ли у него теперь шелл?

/**
 * Функция заливки изображений
 *
 * Раскидывает изображения в папки /month/generateFilename
 * Необязательные параметр - $resolution, ширина и высота. Если они не переданы, просто заливает
 * Возвращает false если это не картинка или если высота, размер или ширина не верны, 
 * Иначе возвращает true;
 *
 * параметр $resolution = array('width'=>'...','height'=>'...')
 */
public function uploadImage($files,$resolution = null, $size = false)
{
	# Если не передан максимальный размер картинки, берём значение из конфига 
	if($size === null)
		$size = Yii::app()->params['imageUpload']['maxSize'];
	
	foreach($files as $key=>$value)
	{
		# Если файл передан в виде $_FILES['Array']['name']['inputname']
		if(is_array($files[$key]['name'])) 
		{
			foreach ($files[$key]['name'] as $k => $v) 
			{
				$name = $k;
			}
			$file['name'] = $files[$key]['name'][$name];
			$file['tmp_name'] = $files[$key]['tmp_name'][$name];
			$file['size'] = $files[$key]['size'][$name];
		}
		# Иначе, обычный $_FILES['inputname']['name']
		else 
		{
			$file['name'] = $files[$key]['name'];
			$file['tmp_name'] = $files[$key]['tmp_name'];
			$file['size'] = $files[$key]['size'];
		}
		# На выходе $file с нужными данными $file = array('name'=>'...','tmp_name'=>'...','size'=>'...')
	} 

	# Если размер превышает допустымый на сервере, возвращаем false
	if($file['size'] > $size*1024)
		return false;

	# Белый список расширений
	$whiteList = array('jpg', 'jpeg', 'png', 'gif');
		
	# Находим расширение файла в нижнем регистре
	$ext = strtolower(pathinfo($file['name'], PATHINFO_EXTENSION));

	# Перебираем массив с допустимыми расширениями
	foreach ($whiteList as $value) {
		# Если совпадение найдено, заливаем изображение
		if($value == $ext)
		{
			# Для функции создания изображения
			if($ext === 'jpg' OR $ext === 'jpeg')
				$function_postfix = 'jpeg';
			else
				$function_postfix = $ext;

			# Находим имя функции
			$function_create = 'imagecreatefrom'.$function_postfix;

			# Сохраняем ресур изображения
			$img = $function_create($file['tmp_name']);

			# Если переданы параметры ширины и высоты изображения, проверяем их
			if($resolution !== null)
			{
				if($resolution['width'] !== imagesx($img))
					return false;
				if($resolution['height'] !== imagesy($img))
					return false;
			}

			# Генерируем имя файла
			$filename = mb_substr(md5(rand(0,99999999).time()), 16).'_'.time().'.'.$ext;

			# текущий месяц для пути
			$current_month = date("F");

			# Определяем путь до изображения
			$way = $_SERVER['DOCUMENT_ROOT'].'/'.Yii::app()->params['imageUpload']['url'].'/'.$current_month;
			
			# Проверяем существование пути до изображения. Если не существует, создаём
			if(!file_exists($way))
				mkdir($way);
			
			# Определяем функцию сохранения изображения
			$function_save = 'image'.$function_postfix;

			# Сохраняем изображение
			$function_save($img,$way.'/'.$filename);

			# Возвращаем путь до изображения
			return $current_month.'/'.$filename;						
		}
	}

	# Если расширение не найдёно, возвращаем false
	return false;
}


Про то, что картинки следует хранить на другом сервере без поддержки PHP я тоже знаю. Меня интересует простое решение для простых смертных. Спасибо
  • Вопрос задан
  • 3022 просмотра
Пригласить эксперта
Ответы на вопрос 3
@Nc_Soft
Как грузить и что грузить это уже издержки перфекционизма, но самое главное:
НИКОГДА НЕ РАЗРЕШАЙТЕ ВЫПОЛНЕНИЕ СКРИПТОВ В ПАПКЕ ДЛЯ ЗАГРУЗКИ
Ответ написан
Комментировать
Sanasol
@Sanasol Куратор тега PHP
нельзя просто так взять и загуглить ошибку
Досконально не смотрел, но первое что бросилось в глаза foreach для разрешенных форматов, жи есть просто. - in_array

И еще какая-то глупость $function_save, непонятно без контекста что это вообще, но мне уже это не нравится.

Зачем createfrom*, вы даже не добавляете водные знаки.
Зачем столько сложностей с именем файла: substr, md5, rand, time, time
Ответ написан
index0h
@index0h
PHP, Golang. https://github.com/index0h
По умолчанию у создаваемого каталога права 0777, т.е вообще все пользователи системы могут делать там все что угодно. Проверьте с какими правами создаются файлы через image*.
Вместо imagecreatefrom* можно использовать stackoverflow.com/questions/15408125/php-check-if-...

Очень рекомендую почитать про PSR-2 и phpDocumentor. Так же разделить логику метода по функциональным составляющим.
Входные параметры на тип и граничные значения тоже неплохо бы проверять, например что будет, если $size задать больше max_upload_size?
Ответ написан
Ваш ответ на вопрос

Войдите, чтобы написать ответ

Похожие вопросы