A
A
Alexey Zorin2015-01-03 02:41:34
PHP
Alexey Zorin, 2015-01-03 02:41:34

Is it safe to upload images like this in PHP?

The function was written for Yii (yes, I know about the existence of extensions and the presence of the yii validator, the question is about the security of this particular function, since it is planned to be extended).
a) Can the code be simplified?
b) Is it safe enough?
c) Suppose a hacker managed to give the server the ability to execute .jpg, .gif and .png files. Does he have a shell now?

/**
 * Функция заливки изображений
 *
 * Раскидывает изображения в папки /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;
}

I also know about the fact that pictures should be stored on another server without PHP support. I'm interested in a simple solution for mere mortals. Thanks

Answer the question

In order to leave comments, you need to log in

3 answer(s)
E
Eugene, 2015-01-03
@Nc_Soft

How to load and what to load is already the cost of perfectionism, but the most important thing:
NEVER ALLOW THE EXECUTION OF SCRIPTS IN THE UPLOAD FOLDER

A
Alexander Aksentiev, 2015-01-03
@Sanasol

I didn’t look thoroughly, but the first thing that caught my eye was foreach for allowed formats, it’s simple. - in_array
And some other nonsense $function_save, it's not clear what it is without context, but I don't like it anymore.
Why createfrom*, you don't even add watermarks.
Why so much complexity with the file name: substr, md5, rand, time, time

I
index0h, 2015-01-03
@index0h

By default, the created directory has rights 0777, that is, in general, all users of the system can do whatever they want there. Check with what rights files are created using image*.
Instead of imagecreatefrom* you can use stackoverflow.com/questions/15408125/php-check-if-...
I highly recommend reading about PSR-2 and phpDocumentor. Also divide the logic of the method into functional components.
It would also be nice to check input parameters for type and boundary values, for example, what happens if $size is set greater than max_upload_size?

Didn't find what you were looking for?

Ask your question

Ask a Question

731 491 924 answers to any question