Скрипт загрузки файла PHP

Я собрал пример сценария загрузки одного файла, который пытается охватить все, что PHP может проверить перед тем, как разрешить успешную загрузку файла. Есть ли что-нибудь еще, доступное в PHP 7.4+, которое я мог бы использовать, чтобы сделать это более безопасным? Например, я использую filter_input ниже, хотя я не нахожу его во многих скриптах.

Взглянем

<?php 

  # EVALUATE REQUEST METHOD
  $REQUEST_METHOD = filter_input(INPUT_SERVER, 'REQUEST_METHOD', FILTER_SANITIZE_ENCODED);

  switch ($REQUEST_METHOD) {

    # HTTP:POST   - PAYLOAD:BLOB
    case 'POST':
      # POST IMAGE
      if(in_array(@$_FILES["files"], $_FILES) && count($_FILES) === 1) {
        upload();
      }
      break;

    default:
      methodInvalid();
      break;
  }

    /**
     * Function upload() uploads a single file.
     * 
     * 
     */
  function upload() {

      // Establish the upload file directory
      $upload_dir =  $_SERVER['DOCUMENT_ROOT'] . '/gui/v1/uploads/submittals/';

      // Establish the upload file path
      $upload_file = $upload_dir . $_FILES['files']['name'][0];

      // Derive the upload file extension
      $upload_file_extension = strtolower(pathinfo($upload_file, PATHINFO_EXTENSION));

      // Allowed file types
      // $allowed_file_extensions = ['pdf', 'jpg', 'jpeg', 'png', 'gif'];
      $allowed_file_extensions = ['pdf'];
      
        /**
         * Does tmp file exist?
         * 
         * 
         */
      if (!file_exists($_FILES['files']['tmp_name'][0])) {

        # ERROR object
        $errorObject             = new stdClass();
        $errorObject->apiVersion = '1.0';
        $errorObject->context="upload.submittal";

        # ABOUT ERROR object
        $aboutError              = new stdClass();
        $aboutError->code="ERR-000";
        $aboutError->message="Select file to upload.";

        # APPEND ABOUT ERROR object TO ERROR object
        $errorObject->error      = $aboutError;

        # RETURN JSON RESPONSE
        header('Content-type:application/json;charset=utf-8');
        return print(json_encode($errorObject));

      }

        /**
         * Is file extension allowed?
         * 
         * 
         */
      if (!in_array($upload_file_extension, $allowed_file_extensions)) {

        # ERROR object
        $errorObject             = new stdClass();
        $errorObject->apiVersion = '1.0';
        $errorObject->context="upload.submittal";

        # ABOUT ERROR object
        $aboutError              = new stdClass();
        $aboutError->code="ERR-000";
        $aboutError->message="Allowed file formats .pdf";

        # APPEND ABOUT ERROR object TO ERROR object
        $errorObject->error      = $aboutError;

        # RETURN JSON RESPONSE
        header('Content-type:application/json;charset=utf-8');
        return print(json_encode($errorObject));

      }

        /**
         * Is file bigger than 20MB?
         * 
         * 
         */
      if ($_FILES['files']['size'][0] > 20000000) {

        # ERROR object
        $errorObject             = new stdClass();
        $errorObject->apiVersion = '1.0';
        $errorObject->context="upload.submittal";

        # ABOUT ERROR object
        $aboutError              = new stdClass();
        $aboutError->code="ERR-000";
        $aboutError->message="File is too large. File size should be less than 20 megabytes.";

        # APPEND ABOUT ERROR object TO ERROR object
        $errorObject->error      = $aboutError;

        # RETURN JSON RESPONSE
        header('Content-type:application/json;charset=utf-8');
        return print(json_encode($errorObject));

      }

        /**
         * Does file already exist?
         * 
         * 
         */
      if (file_exists($upload_file)) {

            /**
             * File overwritten successfuly!
             * 
             * 
             */
        move_uploaded_file($_FILES['files']['tmp_name'][0], $upload_file);

        # SUCCESS object
        $successObject               = new stdClass();
        $successObject->apiVersion   = '1.0';
        $successObject->context="upload.submittal";
        $successObject->status="OK";

        # UPLOAD SUBMITTAL object
        $data                        = new stdClass();
        $data->submittalUploaded     = true;
        # APPEND DATA object TO SUCCESS object
        $successObject->data         = $data;

        # APPEND empty arrays to DATA object
        $successObject->data->arr1   = [];
        $successObject->data->arr2   = [];
        $successObject->data->arr3   = [];

        # RETURN JSON RESPONSE
        header('Content-type:application/json;charset=utf-8');
        return print(json_encode($successObject));

      }

        /**
         * Can file actually be uploaded?
         * 
         * 
         */
      if (!move_uploaded_file($_FILES['files']['tmp_name'][0], $upload_file)) {

            /**
             * File upload error!
             * 
             * 
             */
        # ERROR object
        $errorObject             = new stdClass();
        $errorObject->apiVersion = '1.0';
        $errorObject->context="upload.submittal";

        # ABOUT ERROR object
        $aboutError              = new stdClass();
        $aboutError->code="ERR-000";
        $aboutError->message="File couldn"t be uploaded.';

        # APPEND ABOUT ERROR object TO ERROR object
        $errorObject->error      = $aboutError;

        # RETURN JSON RESPONSE
        header('Content-type:application/json;charset=utf-8');
        return print(json_encode($errorObject));

      } else {

            /**
             * File uploaded successfuly!
             * 
             * 
             */
        # SUCCESS object
        $successObject               = new stdClass();
        $successObject->apiVersion   = '1.0';
        $successObject->context="upload.submittal";
        $successObject->status="OK";

        # UPLOAD SUBMITTAL object
        $data                        = new stdClass();
        $data->submittalUploaded     = true;
        # APPEND DATA object TO SUCCESS object
        $successObject->data         = $data;

        # APPEND empty arrays to DATA object
        $successObject->data->arr1   = [];
        $successObject->data->arr2   = [];
        $successObject->data->arr3   = [];

        # RETURN JSON RESPONSE
        header('Content-type:application/json;charset=utf-8');
        return print(json_encode($successObject));

            // We could insert URL file path to a database from here...

      }

    }

    /**
     * Function methodInvalid() warns about invalid method.
     * 
     * 
     */
  function methodInvalid() {

    # ERROR object
    $errorObject             = new stdClass();
    $errorObject->apiVersion = '1.0';
    $errorObject->context="uploads";

    # ABOUT ERROR object
    $aboutError              = new stdClass();
    $aboutError->code="ERR-000";
    $aboutError->message="Invalid Request. Allowed Methods are POST.";

    # APPEND ABOUT ERROR object TO ERROR object
    $errorObject->error      = $aboutError;

    # RETURN JSON RESPONSE
    header('Content-type:application/json;charset=utf-8');
    return print(json_encode($errorObject));

  }

 ?>

2 ответа
2

Лично мне больше интересно смотреть на возможные проблемы с безопасностью. Загрузка файлов, как известно, опасна, если все сделано неправильно. Уязвимая форма может позволить злоумышленнику загрузить веб-оболочку и захватить ваш сервер.

Давайте сосредоточимся на этой строке:

// Establish the upload file path
$upload_file = $upload_dir . $_FILES['files']['name'][0];

Вы полагаетесь на исходное имя файла, который может быть вредоносным. По определению, это ненадежный сторонний ввод. Что произойдет, если имя файла будет содержать что-то вроде “../../”. Загруженный файл может оказаться за пределами указанной папки. Это классическая атака с обходом пути.

Или что произойдет, если файл содержит пробелы, как насчет “shell.php test.pdf”. Это имя файла будет соответствовать разрешенным расширениям файлов, но что будет дальше? Что возможно могло пойти не так ? Возможно, это не непосредственная уязвимость, но ваш сценарий может легко подавиться неверным именем файла.

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

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

Однако документ говорит:

move_uploaded_file () поддерживает open_basedir. Однако ограничения накладываются только на путь to, чтобы разрешить перемещение загруженных файлов, в которых from может конфликтовать с такими ограничениями. move_uploaded_file () обеспечивает безопасность этой операции, позволяя перемещать только те файлы, которые были загружены через PHP.


Взяв во внимание методы запроса: Боюсь, это спорные соображения безопасности.

Прежде всего, разрешенные методы должны быть установлены в конфигурации вашего веб-сервера или в директивах виртуального хоста. Я не думаю, что ваш сценарий должен заниматься методом (глаголом). Он должен просто ожидать запросов GET или POST. Если будет отправлено что-то еще, ваш сценарий не увидит его и не обработает.

Поэтому я бы настроил веб-сервер для приема GET, POST, а также добавил бы HEAD, потому что поисковые системы могут его использовать.

Документ PHP для move_uploaded_file говорит (курсив мой):

move_uploaded_file (строка $ от, строка $ до): bool

Эта функция проверяет, является ли файл, обозначенный from, допустимым загружаемым файлом (это означает, что он был загружен через механизм загрузки PHP HTTP POST). Если файл действительный, он будет перемещен в имя файла, указанное в.

Это означает, что загрузка файла будет работать только при использовании POST. Сама по себе функция move_uploaded_file достаточна, чтобы утверждать, что загрузка действительно имела место с использованием метода POST.


Что очевидно в вашем коде, так это количество ненужных репетиция. Например, вы проверяете расширение файла, размер файла, а также наличие файла в папке загрузки. Затем вы повторяете материал $ aboutError в каждом блоке, хотя на самом деле единственное, что отличается, – это фактическое сообщение об ошибке, относящееся к проверяемому условию:

$aboutError->message="File is too large. File size should be less than 20 megabytes.";

Даже код ошибки пока везде одинаковый. Код раздут, потому что есть много вещей, которые можно определить только один раз. Вы можете просто переопределить сообщение об ошибке в каждом тесте с помощью соответствующего сообщения и оставить остальные переменные без изменений, поскольку они были установлены ранее.

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


В строке 124: вы проверяете, существует ли уже файл:

if (file_exists($upload_file)) {

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

И на самом деле блок, начинающийся со строки 46, тоже бессмыслен:

if (!file_exists($_FILES['files']['tmp_name'][0])) {

потому что, как упоминалось ранее, move_uploaded_file может проверить, действительно ли существует загруженный файл. Между прочим существует родственная функция: is_uploaded_file

Но вы даже не проверяете, что move_uploaded_file возвращает true – эта функция возвращает логическое значение. Если он не возвращает истину, что-то пошло не так.


Я не понимаю смысла этого кода:

# APPEND empty arrays to DATA object
$successObject->data->arr1   = [];
$successObject->data->arr2   = [];
$successObject->data->arr3   = [];

Если это действительно полезно или имеет смысл, вам следовало бы добавить несколько комментариев, чтобы прояснить цель. Мне даже интересно, действительно ли ваш код работает в его нынешнем виде?


В отступ следует упростить, чтобы сделать схему кода более ясной, особенно блоки управления (ifs или циклы).


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

  • Мне нравится обратная связь. Поводом для повторения было то, что вы ответили так же конкретно, как и вы. Чтобы вы могли видеть и выявлять недостатки так же ясно, как и вы, и в то же время позволять новому программисту легко следовать.

    – такова жизнь


Несколько моих мыслей после быстрого сканирования сверху вниз:

Фильтрация REQUEST_METHOD абсурдно ориентирована на будущее (вы должны предвидеть эволюцию кода, где буквальное содержимое хранится в базе данных и в конечном итоге выводится без проверки в какой-то браузер), но почему-то мне нравится ваше мышление.

Счетчик проверок ($ _ FILES) === 1 вроде как противоположный. Если интерфейс превращается в загрузку второго файла (в другой переменной POST), ваш код игнорирует обе загрузки. Он не вызывает methodInvalid и не регистрирует сообщение об ошибке.

Позже в коде вы дважды проверяете, действительно ли PHP поместил временный файл туда, где он говорит, что поместил его (это излишне, но лучше быть чрезмерно параноиком, чем чрезмерно расслабленным). Но вы не проверяете, что $ _FILES[‘files’][‘name’] содержит неприятные вещи, такие как “../”, которые могут вызвать загрузку файлов в места, куда вы не хотите, чтобы они уходили.

Я заметил множество магических значений, которые появляются внутри вашего кода (поддерживаемые вами расширения, максимальный размер файла, каталог загрузки, коды ошибок, версии API и т. Д.). Вытащите их.

Код не имеет структуры (если вы не учитываете «один оператор за другим» как структуру). Введение функций для изоляции / инкапсуляции функциональных аспектов значительно упрощает поддержку и развитие кода.

Вызов «new stdclass» и присвоение ему поля «apiVersion» подняли бровь. Если вы серьезно относитесь к разработке API, представьте несколько специальных классов для ответов.

Подводя итог: у кода есть много возможностей для улучшений, но я действительно ценю вашу цель сделать его максимально безопасным. В нынешнем виде он выглядит довольно надежно с точки зрения безопасности. Но не игнорируйте тот факт, что текущий низкий уровень удобочитаемости затрудняет / слишком утомляет подлинную проверку того, действительно ли он безопасен.

    Добавить комментарий

    Ваш адрес email не будет опубликован. Обязательные поля помечены *