Unverified Commit fd1d6759 authored by Lee Rowlands's avatar Lee Rowlands
Browse files

Issue #2863652 by alexpott, anmolgoyal74, javi.pl, dww, zaporylie,... 

Issue #2863652 by alexpott, anmolgoyal74, javi.pl, dww, zaporylie, moinak_dutta, iampuma, a.dmitriiev, benjifisher, Pancho, larowlan, yoroy: Do not allow files with "php|pl|py|cgi|asp|js" extensions to be renamed to *.txt and be uploaded if *.txt is not allowed by the field widget
parent 8d2875cc

core/modules/file/file.module

+7 −9
Original line number Diff line number Diff line
@@ -25,6 +25,8 @@ 
use Drupal\Core\Template\Attribute;

use Symfony\Component\Mime\MimeTypeGuesserInterface;



// cspell:ignore btxt



/**

 * The regex pattern used when checking for insecure file types.

 */
@@ -1000,10 +1002,11 @@ function _file_save_upload_single(\SplFileInfo $file_info, $form_field_name, $va 
    // 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))) {

      // If there is no file extension validation at all, or .txt is considered

      // a valid extension and the file would otherwise pass validation, rename

      // it. If the file will be rejected due to extension validation, it should

      // not be renamed; rather, let file_validate_extensions() reject it below.

      if (!isset($validators['file_validate_extensions']) || (preg_match('/\btxt\b/', $extensions) && empty(file_validate_extensions($file, $extensions)))) {

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

        $filename = $file->getFilename();

        if (substr($filename, -4) != '.txt') {
@@ -1012,11 +1015,6 @@ function _file_save_upload_single(\SplFileInfo $file_info, $form_field_name, $va 
        }

        $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/file.post_update.php

0 → 100644
+41 −0
Original line number Diff line number Diff line 
<?php



/**

 * @file

 * Post update functions for File.

 */



use Drupal\Core\Config\Entity\ConfigEntityUpdater;

use Drupal\field\Entity\FieldConfig;

use Drupal\file\Plugin\Field\FieldType\FileItem;



/**

 * Add txt to allowed extensions for all fields that allow uploads of insecure files.

 */

function file_post_update_add_txt_if_allows_insecure_extensions(&$sandbox = NULL) {

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

    return t('The system is configured to allow insecure file uploads. No file field updates are necessary.');

  }



  $updater = function (FieldConfig $field) {

    // Determine if this field uses an item definition that extends FileItem.

    if (is_subclass_of($field->getItemDefinition()->getClass(), FileItem::class)) {

      $allowed_extensions_string = trim($field->getSetting('file_extensions'));

      $allowed_extensions = array_filter(explode(' ', $allowed_extensions_string));

      if (in_array('txt', $allowed_extensions, TRUE)) {

        // Since .txt is specifically allowed, there's nothing to do.

        return FALSE;

      }

      foreach ($allowed_extensions as $extension) {

        // Allow .txt if an insecure extension is allowed.

        if (preg_match(FILE_INSECURE_EXTENSION_REGEX, 'test.' . $extension)) {

          $allowed_extensions_string .= ' txt';

          $field->setSetting('file_extensions', $allowed_extensions_string);

          return TRUE;

        }

      }

      return FALSE;

    }

  };

  \Drupal::classResolver(ConfigEntityUpdater::class)->update($sandbox, 'field_config', $updater);

}

core/modules/file/src/Plugin/Field/FieldType/FileItem.php

+13 −2
Original line number Diff line number Diff line
@@ -225,14 +225,25 @@ public static function validateDirectory($element, FormStateInterface $form_stat 
  public static function validateExtensions($element, FormStateInterface $form_state) {

    if (!empty($element['#value'])) {

      $extensions = preg_replace('/([, ]+\.?)/', ' ', trim(strtolower($element['#value'])));

      $extensions = array_filter(explode(' ', $extensions));

      $extensions = implode(' ', array_unique($extensions));

      $extension_array = array_unique(array_filter(explode(' ', $extensions)));

      $extensions = implode(' ', $extension_array);

      if (!preg_match('/^([a-z0-9]+([.][a-z0-9])* ?)+$/', $extensions)) {

        $form_state->setError($element, t('The list of allowed extensions is not valid, be sure to exclude leading dots and to separate extensions with a comma or space.'));

      }

      else {

        $form_state->setValueForElement($element, $extensions);

      }



      // If insecure uploads are not allowed and txt is not in the list of

      // allowed extensions, ensure that no insecure extensions are allowed.

      if (!in_array('txt', $extension_array, TRUE) && !\Drupal::config('system.file')->get('allow_insecure_uploads')) {

        foreach ($extension_array as $extension) {

          if (preg_match(FILE_INSECURE_EXTENSION_REGEX, 'test.' . $extension)) {

            $form_state->setError($element, t('Add %txt_extension to the list of allowed extensions to securely upload files with a %extension extension. The %txt_extension extension will then be added automatically.', ['%extension' => $extension, '%txt_extension' => 'txt']));

            break;

          }

        }

      }

    }

  }

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

+10 −8
Original line number Diff line number Diff line
@@ -32,6 +32,8 @@ 
use Symfony\Component\HttpFoundation\Request;

use Symfony\Component\HttpKernel\Exception\HttpException;



// cspell:ignore btxt



/**

 * File upload resource.

 *
@@ -485,24 +487,24 @@ protected function prepareFilename($filename, array &$validators) { 
        // 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]));

          // Only allow upload and rename to .txt if .txt files are allowed.

          $passes_validation = $passes_validation && preg_match('/\btxt\b/', $validators['file_validate_extensions'][0]);

        }

        else {

          // All file extensions are allowed.

          $passes_validation = TRUE;

        }

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



        if ($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.

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

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

          }

        }

      }

    }

core/modules/file/tests/src/Functional/FileFieldWidgetTest.php

+33 −0
Original line number Diff line number Diff line
@@ -470,6 +470,39 @@ public function testTemporaryFileRemovalExploitAnonymous() { 
    $this->doTestTemporaryFileRemovalExploit($victim_user, $attacker_user);

  }



  /**

   * Tests configuring file field's allowed file extensions setting.

   */

  public function testFileExtensionsSetting() {

    // Grant the admin user required permissions.

    user_role_grant_permissions($this->adminUser->roles[0]->target_id, ['administer node fields']);



    $type_name = 'article';

    $field_name = strtolower($this->randomMachineName());

    $this->createFileField($field_name, 'node', $type_name);

    $field = FieldConfig::loadByName('node', $type_name, $field_name);

    $field_id = $field->id();



    // By default allowing .php files without .txt is not permitted.

    $this->drupalGet("admin/structure/types/manage/$type_name/fields/$field_id");

    $edit = ['settings[file_extensions]' => 'jpg php'];

    $this->submitForm($edit, 'Save settings');

    $this->assertSession()->pageTextContains('Add txt to the list of allowed extensions to securely upload files with a php extension. The txt extension will then be added automatically.');



    // Test allowing .php and .txt.

    $edit = ['settings[file_extensions]' => 'jpg php txt'];

    $this->submitForm($edit, 'Save settings');

    $this->assertSession()->pageTextContains('Saved ' . $field_name . ' configuration.');



    // If the system is configured to allow insecure uploads, .txt is not

    // required when allowing .php.

    $this->config('system.file')->set('allow_insecure_uploads', TRUE)->save();

    $this->drupalGet("admin/structure/types/manage/$type_name/fields/$field_id");

    $edit = ['settings[file_extensions]' => 'jpg php'];

    $this->submitForm($edit, 'Save settings');

    $this->assertSession()->pageTextContains('Saved ' . $field_name . ' configuration.');

  }



  /**

   * Helper for testing exploiting the temporary file removal using fid.

   *