Commit 104250f4 authored by Jess's avatar Jess
Browse files

SA-CORE-2020-012 by ufku, mrf, fgm, samuel.mortenson, dww, Heine, mlhess,... 

SA-CORE-2020-012 by ufku, mrf, fgm, samuel.mortenson, dww, Heine, mlhess, David_Rothstein, pwolanin, xjm, fgm, stefan.r, dsnopek, rickmanelius, David Strauss, tedbow, alexpott, dww, larowlan, kim.pepper, Wim Leers, quicksketch, mcdruid, Fabianx, effulgentsia, drumm, pandaski, Mixologic

(cherry picked from commit 1994ca5c)
parent 846d1322

core/includes/file.inc

+10 −3
Original line number Diff line number Diff line
@@ -159,8 +159,8 @@ function file_build_uri($path) { 
 * exploit.php_.pps.

 *

 * Specifically, this function adds an underscore to all extensions that are

 * between 2 and 5 characters in length, internal to the file name, and not

 * included in $extensions.

 * between 2 and 5 characters in length, internal to the file name, and either

 * included in the list of unsafe extensions, or not included in $extensions.

 *

 * Function behavior is also controlled by the configuration

 * 'system.file:allow_insecure_uploads'. If it evaluates to TRUE, no alterations
@@ -168,7 +168,8 @@ function file_build_uri($path) { 
 * @param $filename

 *   File name to modify.

 * @param $extensions

 *   A space-separated list of extensions that should not be altered.

 *   A space-separated list of extensions that should not be altered. Note that

 *   extensions that are unsafe will be altered regardless of this parameter.

 * @param $alerts

 *   If TRUE, \Drupal::messenger()->addStatus() will be called to display

 *   a message if the file name was changed.
@@ -187,6 +188,12 @@ function file_munge_filename($filename, $extensions, $alerts = TRUE) { 


    $allowed_extensions = array_unique(explode(' ', strtolower(trim($extensions))));



    // Remove unsafe extensions from the allowed list of extensions.

    // @todo https://www.drupal.org/project/drupal/issues/3032390 Make the list

    //   of unsafe extensions a constant. The list is copied from

    //   FILE_INSECURE_EXTENSION_REGEX.

    $allowed_extensions = array_diff($allowed_extensions, explode('|', 'phar|php|pl|py|cgi|asp|js'));



    // Split the filename up by periods. The first part becomes the basename

    // the last part the final extension.

    $filename_parts = explode('.', $filename);

core/modules/file/file.module

+40 −19
Original line number Diff line number Diff line
@@ -281,7 +281,17 @@ function file_validate(FileInterface $file, $validators = []) { 
  }



  // Let other modules perform validation on the new file.

  return array_merge($errors, \Drupal::moduleHandler()->invokeAll('file_validate', [$file]));

  $errors = array_merge($errors, \Drupal::moduleHandler()->invokeAll('file_validate', [$file]));



  // Ensure the file does not contain a malicious extension. At this point

  // _file_save_upload_single() will have munged the file so it does not contain

  // a malicious extension. Contributed and custom code that calls this method

  // needs to take similar steps if they need to permit files with malicious

  // extensions to be uploaded.

  if (empty($errors) && !\Drupal::config('system.file')->get('allow_insecure_uploads') && preg_match(FILE_INSECURE_EXTENSION_REGEX, $file->getFilename())) {

    $errors[] = t('For security reasons, your upload has been rejected.');

  }

  return $errors;

}



/**
@@ -978,6 +988,8 @@ function _file_save_upload_single(\SplFileInfo $file_info, $form_field_name, $va 
    $validators['file_validate_extensions'][0] = $extensions;

  }



  //  Don't rename if 'allow_insecure_uploads' evaluates to TRUE.

  if (!\Drupal::config('system.file')->get('allow_insecure_uploads')) {

    if (!empty($extensions)) {

      // Munge the filename to protect against possible malicious extension

      // hiding within an unknown file type (ie: filename.html.foo).
@@ -985,18 +997,27 @@ function _file_save_upload_single(\SplFileInfo $file_info, $form_field_name, $va 
    }



    // Rename potentially executable files, to help prevent exploits (i.e. will

  // rename filename.php.foo and filename.php to filename.php.foo.txt and

  // filename.php.txt, respectively). Don't rename if 'allow_insecure_uploads'

  // evaluates to TRUE.

  if (!\Drupal::config('system.file')->get('allow_insecure_uploads') && preg_match(FILE_INSECURE_EXTENSION_REGEX, $file->getFilename()) && (substr($file->getFilename(), -4) != '.txt')) {

    // rename filename.php.foo and filename.php to filename.php_.foo_.txt and

    // filename.php_.txt, respectively).

    if (preg_match(FILE_INSECURE_EXTENSION_REGEX, $file->getFilename())) {

      // If the file will be rejected anyway due to a disallowed extension, it

      // should not be renamed; rather, we'll let file_validate_extensions()

      // reject it below.

      if (!isset($validators['file_validate_extensions']) || empty(file_validate_extensions($file, $extensions))) {

        $file->setMimeType('text/plain');

        $filename = $file->getFilename();

        if (substr($filename, -4) != '.txt') {

          // The destination filename will also later be used to create the URI.

    $file->setFilename($file->getFilename() . '.txt');

    // The .txt extension may not be in the allowed list of extensions. We have

    // to add it here or else the file upload will fail.

    if (!empty($extensions)) {

      $validators['file_validate_extensions'][0] .= ' txt';

          $filename .= '.txt';

        }

        $file->setFilename(file_munge_filename($filename, $extensions));

        \Drupal::messenger()->addStatus(t('For security reasons, your upload has been renamed to %filename.', ['%filename' => $file->getFilename()]));

        // The .txt extension may not be in the allowed list of extensions. We

        // have to add it here or else the file upload will fail.

        if (!empty($validators['file_validate_extensions'][0])) {

          $validators['file_validate_extensions'][0] .= ' txt';

        }

      }

    }

  }

core/modules/file/src/Plugin/rest/resource/FileUploadResource.php

+35 −19
Original line number Diff line number Diff line
@@ -468,6 +468,8 @@ protected function validate(FileInterface $file, array $validators) { 
   *   The prepared/munged filename.

   */

  protected function prepareFilename($filename, array &$validators) {

    // Don't rename if 'allow_insecure_uploads' evaluates to TRUE.

    if (!$this->systemFileConfig->get('allow_insecure_uploads')) {

      if (!empty($validators['file_validate_extensions'][0])) {

        // If there is a file_validate_extensions validator and a list of

        // valid extensions, munge the filename to protect against possible
@@ -476,13 +478,25 @@ protected function prepareFilename($filename, array &$validators) { 
        $filename = file_munge_filename($filename, $validators['file_validate_extensions'][0]);

      }



    // Rename potentially executable files, to help prevent exploits (i.e. will

    // rename filename.php.foo and filename.php to filename.php.foo.txt and

    // filename.php.txt, respectively). Don't rename if 'allow_insecure_uploads'

    // evaluates to TRUE.

    if (!$this->systemFileConfig->get('allow_insecure_uploads') && preg_match(FILE_INSECURE_EXTENSION_REGEX, $filename) && (substr($filename, -4) != '.txt')) {

      // Rename potentially executable files, to help prevent exploits (i.e.

      // will rename filename.php.foo and filename.php to filename._php._foo.txt

      // and filename._php.txt, respectively).

      if (preg_match(FILE_INSECURE_EXTENSION_REGEX, $filename)) {

        // If the file will be rejected anyway due to a disallowed extension, it

        // should not be renamed; rather, we'll let file_validate_extensions()

        // reject it below.

        $passes_validation = FALSE;

        if (!empty($validators['file_validate_extensions'][0])) {

          $file = File::create([]);

          $file->setFilename($filename);

          $passes_validation = empty(file_validate_extensions($file, $validators['file_validate_extensions'][0]));

        }

        if (empty($validators['file_validate_extensions'][0]) || $passes_validation) {

          if ((substr($filename, -4) != '.txt')) {

            // The destination filename will also later be used to create the URI.

            $filename .= '.txt';

          }

          $filename = file_munge_filename($filename, $validators['file_validate_extensions'][0] ?? '');



          // The .txt extension may not be in the allowed list of extensions. We

          // have to add it here or else the file upload will fail.
@@ -490,6 +504,8 @@ protected function prepareFilename($filename, array &$validators) { 
            $validators['file_validate_extensions'][0] .= ' txt';

          }

        }

      }

    }



    return $filename;

  }

core/modules/file/tests/file_test/src/Form/FileTestForm.php

+12 −3
Original line number Diff line number Diff line
@@ -49,9 +49,14 @@ public function buildForm(array $form, FormStateInterface $form_state) { 
    ];



    $form['allow_all_extensions'] = [

      '#type' => 'checkbox',

      '#title' => t('Allow all extensions?'),

      '#default_value' => FALSE,

      '#type' => 'radios',

      '#options' => [

        'false' => 'No',

        'empty_array' => 'Empty array',

        'empty_string' => 'Empty string',

      ],

      '#default_value' => 'false',

    ];



    $form['is_image_file'] = [
@@ -92,9 +97,13 @@ public function submitForm(array &$form, FormStateInterface $form_state) { 
      $validators['file_validate_is_image'] = [];

    }



    if ($form_state->getValue('allow_all_extensions')) {

    $allow = $form_state->getValue('allow_all_extensions');

    if ($allow === 'empty_array') {

      $validators['file_validate_extensions'] = [];

    }

    elseif ($allow === 'empty_string') {

      $validators['file_validate_extensions'] = [''];

    }

    elseif (!$form_state->isValueEmpty('extensions')) {

      $validators['file_validate_extensions'] = [$form_state->getValue('extensions')];

    }

core/modules/file/tests/file_test/src/Form/FileTestSaveUploadFromForm.php

+13 −4
Original line number Diff line number Diff line
@@ -90,9 +90,14 @@ public function buildForm(array $form, FormStateInterface $form_state) { 
    ];



    $form['allow_all_extensions'] = [

      '#type' => 'checkbox',

      '#title' => $this->t('Allow all extensions?'),

      '#default_value' => FALSE,

      '#title' => t('Allow all extensions?'),

      '#type' => 'radios',

      '#options' => [

        'false' => 'No',

        'empty_array' => 'Empty array',

        'empty_string' => 'Empty string',

      ],

      '#default_value' => 'false',

    ];



    $form['is_image_file'] = [
@@ -139,9 +144,13 @@ public function validateForm(array &$form, FormStateInterface $form_state) { 
      $validators['file_validate_is_image'] = [];

    }



    if ($form_state->getValue('allow_all_extensions')) {

    $allow = $form_state->getValue('allow_all_extensions');

    if ($allow === 'empty_array') {

      $validators['file_validate_extensions'] = [];

    }

    elseif ($allow === 'empty_string') {

      $validators['file_validate_extensions'] = [''];

    }

    elseif (!$form_state->isValueEmpty('extensions')) {

      $validators['file_validate_extensions'] = [$form_state->getValue('extensions')];

    }