diff --git a/core/includes/file.inc b/core/includes/file.inc index 33fd9f8b817036737c887be8dcfc0784871473e9..318b32ae8750a0a226e54536c15af74b089cf970 100644 --- a/core/includes/file.inc +++ b/core/includes/file.inc @@ -6,6 +6,7 @@ */ use Drupal\Component\Utility\UrlHelper; +use Drupal\Core\File\FileSystemInterface; use Drupal\Core\StreamWrapper\StreamWrapperManager; /** @@ -176,8 +177,14 @@ function file_build_uri($path) { * * @return string * The potentially modified $filename. + * + * @deprecated in drupal:9.2.0 and is removed from drupal:10.0.0. Dispatch a + * \Drupal\Core\File\Event\FileUploadSanitizeNameEvent event instead. + * + * @see https://www.drupal.org/node/3032541 */ function file_munge_filename($filename, $extensions, $alerts = TRUE) { + @trigger_error('file_munge_filename() is deprecated in drupal:9.2.0 and is removed from drupal:10.0.0. Dispatch a \Drupal\Core\File\Event\FileUploadSanitizeNameEvent event instead. See https://www.drupal.org/node/3032541', E_USER_DEPRECATED); $original = $filename; // Allow potentially insecure uploads for very savvy users and admin @@ -189,10 +196,7 @@ 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')); + $allowed_extensions = array_diff($allowed_extensions, FileSystemInterface::INSECURE_EXTENSIONS); // Split the filename up by periods. The first part becomes the basename // the last part the final extension. @@ -229,8 +233,14 @@ function file_munge_filename($filename, $extensions, $alerts = TRUE) { * * @return * An unmunged filename string. + * + * @deprecated in drupal:9.2.0 and is removed from drupal:10.0.0. Use + * str_replace() instead. + * + * @see https://www.drupal.org/node/3032541 */ function file_unmunge_filename($filename) { + @trigger_error('file_unmunge_filename() is deprecated in drupal:9.2.0 and is removed from drupal:10.0.0. Use str_replace() instead. See https://www.drupal.org/node/3032541', E_USER_DEPRECATED); return str_replace('_.', '.', $filename); } diff --git a/core/lib/Drupal/Core/File/Event/FileUploadSanitizeNameEvent.php b/core/lib/Drupal/Core/File/Event/FileUploadSanitizeNameEvent.php new file mode 100644 index 0000000000000000000000000000000000000000..11102b4c128e81c281a28c1834c5961edd0c441b --- /dev/null +++ b/core/lib/Drupal/Core/File/Event/FileUploadSanitizeNameEvent.php @@ -0,0 +1,125 @@ +<?php + +namespace Drupal\Core\File\Event; + +use Drupal\Component\EventDispatcher\Event; + +/** + * An event during file upload that lets subscribers sanitize the filename. + * + * @see _file_save_upload_single() + * @see \Drupal\file\Plugin\rest\resource\FileUploadResource::prepareFilename() + * @see \Drupal\system\EventSubscriber\SecurityFileUploadEventSubscriber::sanitizeName() + */ +class FileUploadSanitizeNameEvent extends Event { + + /** + * The name of the file being uploaded. + * + * @var string + */ + protected $filename = ''; + + /** + * A list of allowed extensions. + * + * @var string[] + */ + protected $allowedExtensions = []; + + /** + * Indicates the filename has changed for security reasons. + * + * @var bool + */ + protected $isSecurityRename = FALSE; + + /** + * Constructs a file upload sanitize name event object. + * + * @param string $filename + * The full filename (with extension, but not directory) being uploaded. + * @param string $allowed_extensions + * A list of allowed extensions. If empty all extensions are allowed. + */ + public function __construct(string $filename, string $allowed_extensions) { + $this->setFilename($filename); + if ($allowed_extensions !== '') { + $this->allowedExtensions = array_unique(explode(' ', trim(strtolower($allowed_extensions)))); + } + } + + /** + * Gets the filename. + * + * @return string + * The filename. + */ + public function getFilename(): string { + return $this->filename; + } + + /** + * Sets the filename. + * + * @param string $filename + * The filename to use for the uploaded file. + * + * @return $this + * + * @throws \InvalidArgumentException + * Thrown when $filename contains path information. + */ + public function setFilename(string $filename): self { + if (dirname($filename) !== '.') { + throw new \InvalidArgumentException(sprintf('$filename must be a filename with no path information, "%s" provided', $filename)); + } + $this->filename = $filename; + return $this; + } + + /** + * Gets the list of allowed extensions. + * + * @return string[] + * The list of allowed extensions. + */ + public function getAllowedExtensions(): array { + return $this->allowedExtensions; + } + + /** + * Sets the security rename flag. + * + * @return $this + */ + public function setSecurityRename(): self { + $this->isSecurityRename = TRUE; + return $this; + } + + /** + * Gets the security rename flag. + * + * @return bool + * TRUE if there is a rename for security reasons, otherwise FALSE. + */ + public function isSecurityRename(): bool { + return $this->isSecurityRename; + } + + /** + * {@inheritdoc} + * + * @throws \RuntimeException + * Thrown whenever this method is called. This event should always be fully + * processed so that SecurityFileUploadEventSubscriber::sanitizeName() + * gets a chance to run. + * + * @see \Drupal\system\EventSubscriber\SecurityFileUploadEventSubscriber + */ + public function stopPropagation() { + throw new \RuntimeException('Propagation cannot be stopped for the FileUploadSanitizeNameEvent'); + } + +} diff --git a/core/lib/Drupal/Core/File/FileSystemInterface.php b/core/lib/Drupal/Core/File/FileSystemInterface.php index 8294d77d7f9d22ebcd2791a97a4dd2c5d13018c5..cbf8c3f4065c72d99ebbb8e78639661a325cd4cf 100644 --- a/core/lib/Drupal/Core/File/FileSystemInterface.php +++ b/core/lib/Drupal/Core/File/FileSystemInterface.php @@ -32,6 +32,20 @@ interface FileSystemInterface { */ const MODIFY_PERMISSIONS = 2; + /** + * A list of insecure extensions. + * + * @see \Drupal\Core\File\FileSystemInterface::INSECURE_EXTENSION_REGEX + */ + public const INSECURE_EXTENSIONS = ['phar', 'php', 'pl', 'py', 'cgi', 'asp', 'js']; + + /** + * The regex pattern used when checking for insecure file types. + * + * @see \Drupal\Core\File\FileSystemInterface::INSECURE_EXTENSIONS + */ + public const INSECURE_EXTENSION_REGEX = '/\.(phar|php|pl|py|cgi|asp|js)(\.|$)/i'; + /** * Moves an uploaded file to a new location. * diff --git a/core/modules/file/file.api.php b/core/modules/file/file.api.php index d111648b7aa08ee2179d49455413d4d3a5167869..a53c22e7c5dc282327ef0ff94510da99476042a8 100644 --- a/core/modules/file/file.api.php +++ b/core/modules/file/file.api.php @@ -5,6 +5,52 @@ * Hooks for file module. */ +/** + * @addtogroup file + * @{ + * @section file_security Uploading files and security considerations + * + * Using \Drupal\file\Element\ManagedFile field with a defined list of allowed + * extensions is best way to provide a file upload field. It will ensure that: + * - File names are sanitized by the FileUploadSanitizeNameEvent event. + * - Files are validated by hook implementations of hook_file_validate(). + * - Files with insecure extensions will be blocked by default even if they are + * listed. If .txt is an allowed extension such files will be renamed. + * + * The \Drupal\Core\Render\Element\File field requires the developer to ensure + * security concerns are taken care of. To do this, a developer should: + * - Add the #upload_validators property to the form element. For example, + * @code + * $form['file_upload'] = [ + * '#type' => 'file', + * '#title' => $this->t('Upload file'), + * '#upload_validators' => [ + * 'file_validate_extensions' => [ + * 'png gif jpg', + * ], + * ], + * ]; + * @endcode + * - Use file_save_upload() to trigger the FileUploadSanitizeNameEvent event and + * hook_file_validate(). + * + * Important considerations, regardless of the form element used: + * - Always use and validate against a list of allowed extensions. + * - If the configuration system.file:allow_insecure_uploads is set to TRUE + * then potentially insecure files will not be renamed. This setting is not + * recommended. + * + * @see https://cheatsheetseries.owasp.org/cheatsheets/File_Upload_Cheat_Sheet.html + * @see \hook_file_validate() + * @see file_save_upload() + * @see \Drupal\Core\File\Event\FileUploadSanitizeNameEvent + * @see \Drupal\system\EventSubscriber\SecurityFileUploadEventSubscriber + * @see \Drupal\file\Element\ManagedFile + * @see \Drupal\Core\Render\Element\File + * + * @} + */ + /** * @addtogroup hooks * @{ diff --git a/core/modules/file/file.module b/core/modules/file/file.module index 925facef01b9c9bf8fced308bddfea803f39f266..14a4426dd85eefe5c772bdbbbf2e695c671cf72e 100644 --- a/core/modules/file/file.module +++ b/core/modules/file/file.module @@ -18,6 +18,7 @@ use Drupal\Core\Link; use Drupal\Core\Url; use Drupal\file\Entity\File; +use Drupal\Core\File\Event\FileUploadSanitizeNameEvent; use Drupal\file\FileInterface; use Drupal\Component\Utility\NestedArray; use Drupal\Component\Utility\Unicode; @@ -25,12 +26,15 @@ use Drupal\Core\Template\Attribute; use Symfony\Component\Mime\MimeTypeGuesserInterface; -// cspell:ignore btxt - /** * The regex pattern used when checking for insecure file types. + * + * @deprecated in drupal:9.2.0 and is removed from drupal:10.0.0. Use + * \Drupal\Core\File\FileSystemInterface::INSECURE_EXTENSION_REGEX. + * + * @see https://www.drupal.org/node/3032541 */ -define('FILE_INSECURE_EXTENSION_REGEX', '/\.(phar|php|pl|py|cgi|asp|js)(\.|$)/i'); +define('FILE_INSECURE_EXTENSION_REGEX', FileSystemInterface::INSECURE_EXTENSION_REGEX); // Load all Field module hooks for File. require_once __DIR__ . '/file.field.inc'; @@ -290,7 +294,7 @@ function file_validate(FileInterface $file, $validators = []) { // 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())) { + if (empty($errors) && !\Drupal::config('system.file')->get('allow_insecure_uploads') && preg_match(FileSystemInterface::INSECURE_EXTENSION_REGEX, $file->getFilename())) { $errors[] = t('For security reasons, your upload has been rejected.'); } return $errors; @@ -923,7 +927,8 @@ function file_save_upload($form_field_name, $validators = [], $destination = FAL */ function _file_save_upload_single(\SplFileInfo $file_info, $form_field_name, $validators = [], $destination = FALSE, $replace = FileSystemInterface::EXISTS_REPLACE) { $user = \Drupal::currentUser(); - $original_file_name = trim($file_info->getClientOriginalName(), '.'); + // Remember the original filename so we can print a message if it changes. + $original_file_name = $file_info->getClientOriginalName(); // Check for file upload errors and return FALSE for this file if a lower // level system error occurred. For a complete list of errors: // See http://php.net/manual/features.file-upload.errors.php. @@ -951,24 +956,8 @@ function _file_save_upload_single(\SplFileInfo $file_info, $form_field_name, $va return FALSE; } - // Begin building file entity. - $values = [ - 'uid' => $user->id(), - 'status' => 0, - 'filename' => $original_file_name, - 'uri' => $file_info->getRealPath(), - 'filesize' => $file_info->getSize(), - ]; - $guesser = \Drupal::service('file.mime_type.guesser'); - if ($guesser instanceof MimeTypeGuesserInterface) { - $values['filemime'] = $guesser->guessMimeType($values['filename']); - } - else { - $values['filemime'] = $guesser->guess($values['filename']); - @trigger_error('\Symfony\Component\HttpFoundation\File\MimeType\MimeTypeGuesserInterface is deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Implement \Symfony\Component\Mime\MimeTypeGuesserInterface instead. See https://www.drupal.org/node/3133341', E_USER_DEPRECATED); - } - $file = File::create($values); + // Build a list of allowed extensions. $extensions = ''; if (isset($validators['file_validate_extensions'])) { if (isset($validators['file_validate_extensions'][0])) { @@ -984,41 +973,13 @@ function _file_save_upload_single(\SplFileInfo $file_info, $form_field_name, $va } else { // No validator was provided, so add one using the default list. - // Build a default non-munged safe list for file_munge_filename(). + // Build a default non-munged safe list for + // \Drupal\system\EventSubscriber\SecurityFileUploadEventSubscriber::sanitizeName(). $extensions = 'jpg jpeg gif png txt doc xls pdf ppt pps odt ods odp'; $validators['file_validate_extensions'] = []; $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). - $file->setFilename(file_munge_filename($file->getFilename(), $extensions)); - } - - // 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, $file->getFilename())) { - // 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') { - // The destination filename will also later be used to create the URI. - $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()])); - } - } - } - // If the destination is not provided, use the temporary directory. if (empty($destination)) { $destination = 'temporary://'; @@ -1034,22 +995,50 @@ function _file_save_upload_single(\SplFileInfo $file_info, $form_field_name, $va return FALSE; } - $file->source = $form_field_name; // A file URI may already have a trailing slash or look like "public://". if (substr($destination, -1) != '/') { $destination .= '/'; } + + // Call an event to sanitize the filename and to attempt to address security + // issues caused by common server setups. + $event = new FileUploadSanitizeNameEvent($original_file_name, $extensions); + \Drupal::service('event_dispatcher')->dispatch($event); + + // Begin building the file entity. + $values = [ + 'uid' => $user->id(), + 'status' => 0, + // This will be replaced later with a filename based on the destination. + 'filename' => $event->getFilename(), + 'uri' => $file_info->getRealPath(), + 'filesize' => $file_info->getSize(), + ]; + $file = File::create($values); + /** @var \Drupal\Core\File\FileSystemInterface $file_system */ $file_system = \Drupal::service('file_system'); try { - $file->destination = $file_system->getDestinationFilename($destination . $file->getFilename(), $replace); + // Use the result of the sanitization event as the destination name. + $file->destination = $file_system->getDestinationFilename($destination . $event->getFilename(), $replace); } catch (FileException $e) { \Drupal::messenger()->addError(t('The file %filename could not be uploaded because the name is invalid.', ['%filename' => $file->getFilename()])); return FALSE; } - // If the destination is FALSE then there is an existing file and $replace is - // set to return an error, so we need to exit. + + $guesser = \Drupal::service('file.mime_type.guesser'); + if ($guesser instanceof MimeTypeGuesserInterface) { + $file->setMimeType($guesser->guessMimeType($values['filename'])); + } + else { + $file->setMimeType($guesser->guess($values['filename'])); + @trigger_error('\Symfony\Component\HttpFoundation\File\MimeType\MimeTypeGuesserInterface is deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Implement \Symfony\Component\Mime\MimeTypeGuesserInterface instead. See https://www.drupal.org/node/3133341', E_USER_DEPRECATED); + } + $file->source = $form_field_name; + + // If the destination is FALSE then $replace === FILE_EXISTS_ERROR and + // there's an existing file, so we need to bail. if ($file->destination === FALSE) { \Drupal::messenger()->addError(t('The file %source could not be uploaded because a file by that name already exists in the destination %directory.', ['%source' => $form_field_name, '%directory' => $destination])); return FALSE; @@ -1086,6 +1075,21 @@ function _file_save_upload_single(\SplFileInfo $file_info, $form_field_name, $va return FALSE; } + // Update the filename with any changes as a result of the renaming due to an + // existing file. + $file->setFilename(\Drupal::service('file_system')->basename($file->destination)); + + // If the filename has been modified, let the user know. + if ($file->getFilename() !== $original_file_name) { + if ($event->isSecurityRename()) { + $message = t('For security reasons, your upload has been renamed to %filename.', ['%filename' => $file->getFilename()]); + } + else { + $message = t('Your upload has been renamed to %filename.', ['%filename' => $file->getFilename()]); + } + \Drupal::messenger()->addStatus($message); + } + // Set the permissions on the new file. $file_system->chmod($file->getFileUri()); diff --git a/core/modules/file/file.post_update.php b/core/modules/file/file.post_update.php index 707bb4e6b2e6d94427d5aed948fada434592aa88..6d9b26e0b3bde95bff843c4446e61c5bf88354cb 100644 --- a/core/modules/file/file.post_update.php +++ b/core/modules/file/file.post_update.php @@ -6,6 +6,7 @@ */ use Drupal\Core\Config\Entity\ConfigEntityUpdater; +use Drupal\Core\File\FileSystemInterface; use Drupal\field\Entity\FieldConfig; use Drupal\file\Plugin\Field\FieldType\FileItem; @@ -28,7 +29,7 @@ function file_post_update_add_txt_if_allows_insecure_extensions(&$sandbox = NULL } foreach ($allowed_extensions as $extension) { // Allow .txt if an insecure extension is allowed. - if (preg_match(FILE_INSECURE_EXTENSION_REGEX, 'test.' . $extension)) { + if (preg_match(FileSystemInterface::INSECURE_EXTENSION_REGEX, 'test.' . $extension)) { $allowed_extensions_string .= ' txt'; $field->setSetting('file_extensions', $allowed_extensions_string); return TRUE; diff --git a/core/modules/file/src/Plugin/Field/FieldType/FileItem.php b/core/modules/file/src/Plugin/Field/FieldType/FileItem.php index 6fa9b51993498991fcf5a37b26d7118a4380fb72..a6ca60943ff039931737c09dd0033ecbdda22b8b 100644 --- a/core/modules/file/src/Plugin/Field/FieldType/FileItem.php +++ b/core/modules/file/src/Plugin/Field/FieldType/FileItem.php @@ -238,7 +238,7 @@ public static function validateExtensions($element, FormStateInterface $form_sta // 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)) { + if (preg_match(FileSystemInterface::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; } diff --git a/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php b/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php index 399d55f2a78316cea7b652b7a291677a2d0718ad..d46a8b20b0cae41acaf7447c85291f7646573ac5 100644 --- a/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php +++ b/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php @@ -14,6 +14,7 @@ use Drupal\Core\Session\AccountInterface; use Drupal\Core\Utility\Token; use Drupal\file\FileInterface; +use Drupal\Core\File\Event\FileUploadSanitizeNameEvent; use Drupal\rest\ModifiedResourceResponse; use Drupal\rest\Plugin\ResourceBase; use Drupal\Component\Render\PlainTextOutput; @@ -23,6 +24,7 @@ use Drupal\rest\RequestHandler; use Psr\Log\LoggerInterface; use Symfony\Component\DependencyInjection\ContainerInterface; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; @@ -32,8 +34,6 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Exception\HttpException; -// cspell:ignore btxt - /** * File upload resource. * @@ -128,6 +128,13 @@ class FileUploadResource extends ResourceBase { */ protected $systemFileConfig; + /** + * The event dispatcher to dispatch the filename sanitize event. + * + * @var \Symfony\Component\EventDispatcher\EventDispatcherInterface + */ + protected $eventDispatcher; + /** * Constructs a FileUploadResource instance. * @@ -157,8 +164,10 @@ class FileUploadResource extends ResourceBase { * The lock service. * @param \Drupal\Core\Config\Config $system_file_config * The system file configuration. + * @param \Symfony\Component\EventDispatcher\EventDispatcherInterface $event_dispatcher + * The event dispatcher service. */ - public function __construct(array $configuration, $plugin_id, $plugin_definition, $serializer_formats, LoggerInterface $logger, FileSystemInterface $file_system, EntityTypeManagerInterface $entity_type_manager, EntityFieldManagerInterface $entity_field_manager, AccountInterface $current_user, $mime_type_guesser, Token $token, LockBackendInterface $lock, Config $system_file_config) { + public function __construct(array $configuration, $plugin_id, $plugin_definition, $serializer_formats, LoggerInterface $logger, FileSystemInterface $file_system, EntityTypeManagerInterface $entity_type_manager, EntityFieldManagerInterface $entity_field_manager, AccountInterface $current_user, $mime_type_guesser, Token $token, LockBackendInterface $lock, Config $system_file_config, EventDispatcherInterface $event_dispatcher = NULL) { parent::__construct($configuration, $plugin_id, $plugin_definition, $serializer_formats, $logger); $this->fileSystem = $file_system; $this->entityTypeManager = $entity_type_manager; @@ -168,6 +177,11 @@ public function __construct(array $configuration, $plugin_id, $plugin_definition $this->token = $token; $this->lock = $lock; $this->systemFileConfig = $system_file_config; + if (!$event_dispatcher) { + @trigger_error('The event dispatcher service should be passed to FileUploadResource::__construct() since 9.2.0. This will be required in Drupal 10.0.0. See https://www.drupal.org/node/3032541', E_USER_DEPRECATED); + $event_dispatcher = \Drupal::service('event_dispatcher'); + } + $this->eventDispatcher = $event_dispatcher; } /** @@ -187,7 +201,8 @@ public static function create(ContainerInterface $container, array $configuratio $container->get('file.mime_type.guesser'), $container->get('token'), $container->get('lock'), - $container->get('config.factory')->get('system.file') + $container->get('config.factory')->get('system.file'), + $container->get('event_dispatcher') ); } @@ -470,46 +485,12 @@ 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 - // malicious extension hiding within an unknown file type. For example, - // "filename.html.foo". - $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). - 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. - 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 ($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] ?? ''); - } - } - } - - return $filename; + // The actual extension validation occurs in + // \Drupal\file\Plugin\rest\resource\FileUploadResource::validate(). + $extensions = $validators['file_validate_extensions'][0] ?? ''; + $event = new FileUploadSanitizeNameEvent($filename, $extensions); + $this->eventDispatcher->dispatch($event); + return $event->getFilename(); } /** diff --git a/core/modules/file/tests/src/Functional/SaveUploadTest.php b/core/modules/file/tests/src/Functional/SaveUploadTest.php index fa121eab4ce2595053348e907a21c635194c1469..fb3b73a10bfc37205800466efcff40a45b794ce4 100644 --- a/core/modules/file/tests/src/Functional/SaveUploadTest.php +++ b/core/modules/file/tests/src/Functional/SaveUploadTest.php @@ -337,6 +337,63 @@ public function testHandleDangerousFile() { $this->assertFileHooksCalled(['validate']); } + /** + * Test dangerous file handling. + */ + public function testHandleDotFile() { + $dot_file = $this->siteDirectory . '/.test'; + file_put_contents($dot_file, 'This is a test'); + $config = $this->config('system.file'); + $edit = [ + 'file_test_replace' => FileSystemInterface::EXISTS_REPLACE, + 'files[file_test_upload]' => \Drupal::service('file_system')->realpath($dot_file), + 'is_image_file' => FALSE, + ]; + $this->drupalGet('file-test/upload'); + $this->submitForm($edit, 'Submit'); + $this->assertSession()->statusCodeEquals(200); + $this->assertSession()->pageTextContains('The specified file .test could not be uploaded'); + $this->assertSession()->pageTextContains('Epic upload FAIL!'); + + // Check that the correct hooks were called. + $this->assertFileHooksCalled(['validate']); + + $edit = [ + 'file_test_replace' => FileSystemInterface::EXISTS_RENAME, + 'files[file_test_upload]' => \Drupal::service('file_system')->realpath($dot_file), + 'is_image_file' => FALSE, + 'allow_all_extensions' => 'empty_array', + ]; + $this->drupalGet('file-test/upload'); + $this->submitForm($edit, 'Submit'); + $this->assertSession()->statusCodeEquals(200); + $this->assertSession()->pageTextContains('For security reasons, your upload has been renamed to test.'); + $this->assertSession()->pageTextContains('File name is test.'); + $this->assertSession()->pageTextContains('You WIN!'); + + // Check that the correct hooks were called. + $this->assertFileHooksCalled(['validate', 'insert']); + + // Reset the hook counters. + file_test_reset(); + + // Turn off insecure uploads, then try the same thing as above to ensure dot + // files are renamed regardless. + $config->set('allow_insecure_uploads', 0)->save(); + $this->drupalGet('file-test/upload'); + $this->submitForm($edit, 'Submit'); + $this->assertSession()->statusCodeEquals(200); + $this->assertSession()->pageTextContains('For security reasons, your upload has been renamed to test_0.'); + $this->assertSession()->pageTextContains('File name is test_0.'); + $this->assertSession()->pageTextContains('You WIN!'); + + // Check that the correct hooks were called. + $this->assertFileHooksCalled(['validate', 'insert']); + + // Reset the hook counters. + file_test_reset(); + } + /** * Test file munge handling. */ @@ -422,7 +479,7 @@ public function testHandleFileMunge() { $this->drupalPostForm('file-test/upload', $edit, 'Submit'); $this->assertSession()->statusCodeEquals(200); $this->assertRaw(t('For security reasons, your upload has been renamed')); - $this->assertRaw(t('File name is @filename', ['@filename' => 'image-test.png.php_.png'])); + $this->assertRaw(t('File name is @filename', ['@filename' => 'image-test.png_.php_.png'])); $this->assertRaw(t('You WIN!')); // Check that the correct hooks were called. @@ -440,7 +497,7 @@ public function testHandleFileMunge() { $this->drupalPostForm('file-test/upload', $edit, 'Submit'); $this->assertSession()->statusCodeEquals(200); $this->assertRaw(t('For security reasons, your upload has been renamed')); - $this->assertRaw(t('File name is @filename.', ['@filename' => 'image-test.png_.php_.png_.txt'])); + $this->assertRaw(t('File name is @filename.', ['@filename' => 'image-test.png_.php__0.png'])); $this->assertRaw(t('You WIN!')); // Check that the correct hooks were called. diff --git a/core/modules/jsonapi/src/Controller/TemporaryJsonapiFileFieldUploader.php b/core/modules/jsonapi/src/Controller/TemporaryJsonapiFileFieldUploader.php index fd60f11f374c5326a7715780a267eb1e3e7b5a1d..4751d9caaca325bf3d3165c33823a2708c72ae81 100644 --- a/core/modules/jsonapi/src/Controller/TemporaryJsonapiFileFieldUploader.php +++ b/core/modules/jsonapi/src/Controller/TemporaryJsonapiFileFieldUploader.php @@ -8,6 +8,7 @@ use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\Plugin\DataType\EntityAdapter; use Drupal\Core\Field\FieldDefinitionInterface; +use Drupal\Core\File\Event\FileUploadSanitizeNameEvent; use Drupal\Core\File\Exception\FileException; use Drupal\Core\Validation\DrupalTranslator; use Drupal\file\FileInterface; @@ -26,8 +27,7 @@ use Symfony\Component\HttpKernel\Exception\HttpException; use Symfony\Component\Mime\MimeTypeGuesserInterface; use Symfony\Component\Validator\ConstraintViolation; - -// cspell:ignore btxt +use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; /** * Reads data from an upload stream and creates a corresponding file entity. @@ -100,6 +100,13 @@ class TemporaryJsonapiFileFieldUploader { */ protected $systemFileConfig; + /** + * The event dispatcher. + * + * @var \Symfony\Contracts\EventDispatcher\EventDispatcherInterface + */ + protected $eventDispatcher; + /** * Constructs a FileUploadResource instance. * @@ -116,13 +123,17 @@ class TemporaryJsonapiFileFieldUploader { * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory * The config factory. */ - public function __construct(LoggerInterface $logger, FileSystemInterface $file_system, $mime_type_guesser, Token $token, LockBackendInterface $lock, ConfigFactoryInterface $config_factory) { + public function __construct(LoggerInterface $logger, FileSystemInterface $file_system, $mime_type_guesser, Token $token, LockBackendInterface $lock, ConfigFactoryInterface $config_factory, EventDispatcherInterface $event_dispatcher = NULL) { $this->logger = $logger; $this->fileSystem = $file_system; $this->mimeTypeGuesser = $mime_type_guesser; $this->token = $token; $this->lock = $lock; $this->systemFileConfig = $config_factory->get('system.file'); + if (!$event_dispatcher) { + $event_dispatcher = \Drupal::service('event_dispatcher'); + } + $this->eventDispatcher = $event_dispatcher; } /** @@ -388,46 +399,12 @@ 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 - // malicious extension hiding within an unknown file type. For example, - // "filename.html.foo". - $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). - 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. - 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 ($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] ?? ''); - } - } - } - - return $filename; + // The actual extension validation occurs in + // \Drupal\jsonapi\Controller\TemporaryJsonapiFileFieldUploader::validate(). + $extensions = $validators['file_validate_extensions'][0] ?? ''; + $event = new FileUploadSanitizeNameEvent($filename, $extensions); + $this->eventDispatcher->dispatch($event); + return $event->getFilename(); } /** diff --git a/core/modules/jsonapi/tests/src/Functional/FileUploadTest.php b/core/modules/jsonapi/tests/src/Functional/FileUploadTest.php index 39299b4a54a12f690e6e35d75cfd06cba584be7a..e0c2408017806fdebb5717829f6f329de6702603 100644 --- a/core/modules/jsonapi/tests/src/Functional/FileUploadTest.php +++ b/core/modules/jsonapi/tests/src/Functional/FileUploadTest.php @@ -678,13 +678,13 @@ public function testFileUploadMaliciousExtension() { $this->field->setSetting('file_extensions', '')->save(); $this->rebuildAll(); $response = $this->fileRequest($uri, $php_string, ['Content-Disposition' => 'filename="example_5.php.png"']); - $expected = $this->getExpectedDocument(5, 'example_5.php_.png_.txt', TRUE); + $expected = $this->getExpectedDocument(5, 'example_5.php_.png', TRUE); // Override the expected filesize. $expected['data']['attributes']['filesize'] = strlen($php_string); - // The file mime should also now be text. - $expected['data']['attributes']['filemime'] = 'text/plain'; + // The file mime should still see this as a PNG image. + $expected['data']['attributes']['filemime'] = 'image/png'; $this->assertResponseData($expected, $response); - $this->assertFileExists('public://foobar/example_5.php_.png_.txt'); + $this->assertFileExists('public://foobar/example_5.php_.png'); // Dangerous extensions are munged if is renamed to end in .txt. $response = $this->fileRequest($uri, $php_string, ['Content-Disposition' => 'filename="example_6.cgi.png.txt"']); diff --git a/core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php b/core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php index e66259854313f6e2fca89eb7242a620667df4910..a1c4a74f8db973e286b9e7becda75040988e518d 100644 --- a/core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php @@ -574,13 +574,13 @@ public function testFileUploadMaliciousExtension() { $this->field->setSetting('file_extensions', '')->save(); $this->rebuildAll(); $response = $this->fileRequest($uri, $php_string, ['Content-Disposition' => 'filename="example_5.php.png"']); - $expected = $this->getExpectedNormalizedEntity(5, 'example_5.php_.png_.txt', TRUE); + $expected = $this->getExpectedNormalizedEntity(5, 'example_5.php_.png', TRUE); // Override the expected filesize. $expected['filesize'][0]['value'] = strlen($php_string); - // The file mime should also now be text. - $expected['filemime'][0]['value'] = 'text/plain'; + // The file mime should still see this as a PNG image. + $expected['filemime'][0]['value'] = 'image/png'; $this->assertResponseData($expected, $response); - $this->assertFileExists('public://foobar/example_5.php_.png_.txt'); + $this->assertFileExists('public://foobar/example_5.php_.png'); // Dangerous extensions are munged if is renamed to end in .txt. $response = $this->fileRequest($uri, $php_string, ['Content-Disposition' => 'filename="example_6.cgi.png.txt"']); diff --git a/core/modules/system/src/EventSubscriber/SecurityFileUploadEventSubscriber.php b/core/modules/system/src/EventSubscriber/SecurityFileUploadEventSubscriber.php new file mode 100644 index 0000000000000000000000000000000000000000..4aeeb574d35e690836bd4705100e61687e50dfce --- /dev/null +++ b/core/modules/system/src/EventSubscriber/SecurityFileUploadEventSubscriber.php @@ -0,0 +1,122 @@ +<?php + +namespace Drupal\system\EventSubscriber; + +use Drupal\Core\Config\ConfigFactoryInterface; +use Drupal\Core\File\Event\FileUploadSanitizeNameEvent; +use Drupal\Core\File\FileSystemInterface; +use Symfony\Component\EventDispatcher\EventSubscriberInterface; + +/** + * The final subscriber to 'file.upload.sanitize.name'. + * + * This prevents insecure filenames. + */ +class SecurityFileUploadEventSubscriber implements EventSubscriberInterface { + + /** + * The system.file configuration. + * + * @var \Drupal\Core\Config\Config + */ + protected $config; + + /** + * Constructs a new file event listener. + * + * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory + * The config factory. + */ + public function __construct(ConfigFactoryInterface $config_factory) { + $this->config = $config_factory->get('system.file'); + } + + /** + * {@inheritdoc} + */ + public static function getSubscribedEvents() { + // This event must be run last to ensure the filename obeys the security + // rules. + $events[FileUploadSanitizeNameEvent::class][] = ['sanitizeName', PHP_INT_MIN]; + return $events; + } + + /** + * Sanitizes the upload's filename to make it secure. + * + * @param \Drupal\Core\File\Event\FileUploadSanitizeNameEvent $event + * File upload sanitize name event. + */ + public function sanitizeName(FileUploadSanitizeNameEvent $event): void { + $filename = $event->getFilename(); + // Dot files are renamed regardless of security settings. + $filename = trim($filename, '.'); + + // Remove any null bytes. See + // http://php.net/manual/security.filesystem.nullbytes.php + $filename = str_replace(chr(0), '', $filename); + + // Split up the filename by periods. The first part becomes the basename, + // the last part the final extension. + $filename_parts = explode('.', $filename); + // Remove file basename. + $filename = array_shift($filename_parts); + // Remove final extension. + $final_extension = (string) array_pop($filename_parts); + + $extensions = $event->getAllowedExtensions(); + if (!empty($extensions) && !in_array(strtolower($final_extension), $extensions, TRUE)) { + // This upload will be rejected by file_validate_extensions() anyway so do + // not make any alterations to the filename. This prevents a file named + // 'example.php' being renamed to 'example.php_.txt' and uploaded if the + // .txt extension is allowed but .php is not. It is the responsibility of + // the function that dispatched the event to ensure file_validate() is + // called with 'file_validate_extensions' in the list of validators if + // $extensions is not empty. + return; + } + + if (!$this->config->get('allow_insecure_uploads') && in_array(strtolower($final_extension), FileSystemInterface::INSECURE_EXTENSIONS, TRUE)) { + if (empty($extensions) || in_array('txt', $extensions, TRUE)) { + // Add .txt to potentially executable files prior to munging to help prevent + // exploits. This results in a filenames like filename.php being changed to + // filename.php.txt prior to munging. + $filename_parts[] = $final_extension; + $final_extension = 'txt'; + } + else { + // Since .txt is not an allowed extension do not rename the file. The + // file will be rejected by file_validate(). + return; + } + } + + // If there are any insecure extensions in the filename munge all the + // internal extensions. + $munge_everything = !empty(array_intersect(array_map('strtolower', $filename_parts), FileSystemInterface::INSECURE_EXTENSIONS)); + + // Munge the filename to protect against possible malicious extension hiding + // within an unknown file type (i.e. filename.html.foo). This was introduced + // as part of SA-2006-006 to fix Apache's risky fallback behaviour. + + // Loop through the middle parts of the name and add an underscore to the + // end of each section that could be a file extension but isn't in the + // list of allowed extensions. + foreach ($filename_parts as $filename_part) { + $filename .= '.' . $filename_part; + if ($munge_everything) { + $filename .= '_'; + } + elseif (!empty($extensions) && !in_array(strtolower($filename_part), $extensions) && preg_match("/^[a-zA-Z]{2,5}\d?$/", $filename_part)) { + $filename .= '_'; + } + } + if ($final_extension !== '') { + $filename .= '.' . $final_extension; + } + if ($filename !== $event->getFilename()) { + $event->setFilename($filename)->setSecurityRename(); + } + } + +} diff --git a/core/modules/system/system.services.yml b/core/modules/system/system.services.yml index 9db3db1a5af4b296fac11a51d9d20cfd9cfc3f86..c0e4d605eab2b3d9d9214c435c3085c29d0847bb 100644 --- a/core/modules/system/system.services.yml +++ b/core/modules/system/system.services.yml @@ -48,3 +48,8 @@ services: arguments: ['@current_user', '@config.factory'] tags: - { name: event_subscriber } + system.file_event.subscriber: + class: Drupal\system\EventSubscriber\SecurityFileUploadEventSubscriber + arguments: ['@config.factory'] + tags: + - { name: event_subscriber } diff --git a/core/modules/system/tests/src/Unit/Event/SecurityFileUploadEventSubscriberTest.php b/core/modules/system/tests/src/Unit/Event/SecurityFileUploadEventSubscriberTest.php new file mode 100644 index 0000000000000000000000000000000000000000..f51f750b42b87f065bddbb43331c4c470b75a62d --- /dev/null +++ b/core/modules/system/tests/src/Unit/Event/SecurityFileUploadEventSubscriberTest.php @@ -0,0 +1,150 @@ +<?php + +namespace Drupal\Tests\system\Unit\Event; + +use Drupal\Core\File\Event\FileUploadSanitizeNameEvent; +use Drupal\system\EventSubscriber\SecurityFileUploadEventSubscriber; +use Drupal\Tests\UnitTestCase; + +/** + * SecurityFileUploadEventSubscriber tests. + * + * @group system + * @coversDefaultClass \Drupal\system\EventSubscriber\SecurityFileUploadEventSubscriber + */ +class SecurityFileUploadEventSubscriberTest extends UnitTestCase { + + /** + * Tests file name sanitization. + * + * @param string $filename + * The original filename. + * @param string $allowed_extensions + * The allowed extensions. + * @param string $expected_filename + * The expected filename if 'allow_insecure_uploads' is set to FALSE. + * @param string|null $expected_filename_with_insecure_uploads + * The expected filename if 'allow_insecure_uploads' is set to TRUE. + * + * @dataProvider provideFilenames + * + * @covers ::sanitizeName + */ + public function testSanitizeName(string $filename, string $allowed_extensions, string $expected_filename, string $expected_filename_with_insecure_uploads = NULL) { + // Configure insecure uploads to be renamed. + $config_factory = $this->getConfigFactoryStub([ + 'system.file' => [ + 'allow_insecure_uploads' => FALSE, + ], + ]); + + $subscriber = new SecurityFileUploadEventSubscriber($config_factory); + $event = new FileUploadSanitizeNameEvent($filename, $allowed_extensions); + $subscriber->sanitizeName($event); + + // Check the results of the configured sanitization. + $this->assertSame($expected_filename, $event->getFilename()); + $this->assertSame($expected_filename !== $filename, $event->isSecurityRename()); + + // Rerun the event allowing insecure uploads. + $config_factory = $this->getConfigFactoryStub([ + 'system.file' => [ + 'allow_insecure_uploads' => TRUE, + ], + ]); + + $subscriber = new SecurityFileUploadEventSubscriber($config_factory); + $event = new FileUploadSanitizeNameEvent($filename, $allowed_extensions); + $subscriber->sanitizeName($event); + + // Check the results of the configured sanitization. + $expected_filename_with_insecure_uploads = $expected_filename_with_insecure_uploads ?? $expected_filename; + $this->assertSame($expected_filename_with_insecure_uploads, $event->getFilename()); + $this->assertSame($expected_filename_with_insecure_uploads !== $filename, $event->isSecurityRename()); + } + + /** + * Provides data for testSanitizeName(). + * + * @return array + * Arrays with original name, allowed extensions, expected name and + * (optional) expected name 'allow_insecure_uploads' is set to TRUE. + */ + public function provideFilenames() { + return [ + 'All extensions allowed filename not munged' => ['foo.txt', '', 'foo.txt'], + 'All extensions allowed with .php file' => ['foo.php', '', 'foo.php_.txt', 'foo.php'], + 'All extensions allowed with .pHp file' => ['foo.pHp', '', 'foo.pHp_.txt', 'foo.pHp'], + 'All extensions allowed with .PHP file' => ['foo.PHP', '', 'foo.PHP_.txt', 'foo.PHP'], + '.php extension allowed with .php file' => ['foo.php', 'php', 'foo.php', 'foo.php'], + '.PhP extension allowed with .php file' => ['foo.php', 'PhP', 'foo.php', 'foo.php'], + '.php, .txt extension allowed with .php file' => ['foo.php', 'php txt', 'foo.php_.txt', 'foo.php'], + '.PhP, .tXt extension allowed with .php file' => ['foo.php', 'PhP tXt', 'foo.php_.txt', 'foo.php'], + 'no extension produces no errors' => ['foo', '', 'foo'], + 'filename is munged' => ['foo.phar.png.php.jpg', 'jpg png', 'foo.phar_.png_.php_.jpg'], + 'filename is munged regardless of case' => ['FOO.pHAR.PNG.PhP.jpg', 'jpg png', 'FOO.pHAR_.PNG_.PhP_.jpg'], + 'null bytes are removed' => ['foo' . chr(0) . '.txt' . chr(0), '', 'foo.txt'], + 'dot files are renamed' => ['.htaccess', '', 'htaccess'], + ]; + } + + /** + * Tests file name sanitization without file munging. + * + * @param string $filename + * The original filename. + * @param string $allowed_extensions + * The allowed extensions. + * + * @dataProvider provideFilenamesNoMunge + * + * @covers ::sanitizeName + */ + public function testSanitizeNameNoMunge(string $filename, string $allowed_extensions) { + $config_factory = $this->getConfigFactoryStub([ + 'system.file' => [ + 'allow_insecure_uploads' => FALSE, + ], + ]); + + $subscriber = new SecurityFileUploadEventSubscriber($config_factory); + $event = new FileUploadSanitizeNameEvent($filename, $allowed_extensions); + $subscriber->sanitizeName($event); + + // Check the results of the configured sanitization. + $this->assertSame($filename, $event->getFilename()); + $this->assertSame(FALSE, $event->isSecurityRename()); + + $config_factory = $this->getConfigFactoryStub([ + 'system.file' => [ + 'allow_insecure_uploads' => TRUE, + ], + ]); + + $event = new FileUploadSanitizeNameEvent($filename, $allowed_extensions); + $subscriber = new SecurityFileUploadEventSubscriber($config_factory); + $subscriber->sanitizeName($event); + + // Check the results of the configured sanitization. + $this->assertSame($filename, $event->getFilename()); + $this->assertSame(FALSE, $event->isSecurityRename()); + } + + /** + * Provides data for testSanitizeNameNoMunge(). + * + * @return array + * Arrays with original name and allowed extensions. + */ + public function provideFilenamesNoMunge() { + return [ + // The following filename would be rejected by file_validate_extension() + // and therefore remains unchanged. + '.php is not munged when it would be rejected' => ['foo.php.php', 'jpg'], + '.php is not munged when it would be rejected and filename contains null byte character' => ['foo.' . chr(0) . 'php.php', 'jpg'], + 'extension less files are not munged when they would be rejected' => ['foo', 'jpg'], + 'dot files are not munged when they would be rejected' => ['.htaccess', 'jpg png'], + ]; + } + +} diff --git a/core/tests/Drupal/KernelTests/Core/File/NameMungingTest.php b/core/tests/Drupal/KernelTests/Core/File/NameMungingTest.php index f7028cf1e885c58b5118253dfeb2b9ba9d1193f7..e85512c62c4edbf757c3bba208b4ca0394a8b660 100644 --- a/core/tests/Drupal/KernelTests/Core/File/NameMungingTest.php +++ b/core/tests/Drupal/KernelTests/Core/File/NameMungingTest.php @@ -25,6 +25,7 @@ * - (name).foo.txt -> (unchecked) -> (name).foo.txt after un-munging * * @group File + * @group legacy */ class NameMungingTest extends FileTestBase { @@ -60,6 +61,7 @@ protected function setUp(): void { * Create a file and munge/unmunge the name. */ public function testMunging() { + $this->expectDeprecation('file_munge_filename() is deprecated in drupal:9.2.0 and is removed from drupal:10.0.0. Dispatch a \Drupal\Core\File\Event\FileUploadSanitizeNameEvent event instead. See https://www.drupal.org/node/3032541'); // Disable insecure uploads. $this->config('system.file')->set('allow_insecure_uploads', 0)->save(); $munged_name = file_munge_filename($this->name, '', TRUE); @@ -117,6 +119,7 @@ public function testMungeUnsafe() { * Ensure that unmunge gets your name back. */ public function testUnMunge() { + $this->expectDeprecation('file_unmunge_filename() is deprecated in drupal:9.2.0 and is removed from drupal:10.0.0. Use str_replace() instead. See https://www.drupal.org/node/3032541'); $munged_name = file_munge_filename($this->name, '', FALSE); $unmunged_name = file_unmunge_filename($munged_name); $this->assertSame($unmunged_name, $this->name, new FormattableMarkup('The unmunged (%unmunged) filename matches the original (%original)', ['%unmunged' => $unmunged_name, '%original' => $this->name])); diff --git a/core/tests/Drupal/Tests/Core/File/FileUploadSanitizeNameEventTest.php b/core/tests/Drupal/Tests/Core/File/FileUploadSanitizeNameEventTest.php new file mode 100644 index 0000000000000000000000000000000000000000..64fbd778236ae030b3881aec6e46ce14b0400896 --- /dev/null +++ b/core/tests/Drupal/Tests/Core/File/FileUploadSanitizeNameEventTest.php @@ -0,0 +1,111 @@ +<?php + +namespace Drupal\Tests\Core\File; + +use Drupal\Core\File\Event\FileUploadSanitizeNameEvent; +use Drupal\Tests\UnitTestCase; + +// cspell:ignore äöüåøhello + +/** + * FileUploadSanitizeNameEvent tests. + * + * @group file + * @coversDefaultClass \Drupal\Core\File\Event\FileUploadSanitizeNameEvent + */ +class FileUploadSanitizeNameEventTest extends UnitTestCase { + + /** + * @covers ::setFilename + * @covers ::getFilename + */ + public function testSetFilename() { + $event = new FileUploadSanitizeNameEvent('foo.txt', ''); + $this->assertSame('foo.txt', $event->getFilename()); + $event->setFilename('foo.html'); + $this->assertSame('foo.html', $event->getFilename()); + } + + /** + * @covers ::setFilename + */ + public function testSetFilenameException() { + $event = new FileUploadSanitizeNameEvent('foo.txt', ''); + $this->assertSame('foo.txt', $event->getFilename()); + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('$filename must be a filename with no path information, "bar/foo.html" provided'); + $event->setFilename('bar/foo.html'); + } + + /** + * @covers ::__construct + * @covers ::setFilename + */ + public function testConstructorException() { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('$filename must be a filename with no path information, "bar/foo.txt" provided'); + new FileUploadSanitizeNameEvent('bar/foo.txt', ''); + } + + /** + * @covers ::getAllowedExtensions + */ + public function testAllowedExtensions() { + $event = new FileUploadSanitizeNameEvent('foo.txt', ''); + $this->assertSame([], $event->getAllowedExtensions()); + + $event = new FileUploadSanitizeNameEvent('foo.txt', 'gif png'); + $this->assertSame(['gif', 'png'], $event->getAllowedExtensions()); + } + + /** + * Test event construction. + * + * @dataProvider provideFilenames + * @covers ::__construct + * @covers ::getFilename + * + * @param string $filename + * The filename to test + */ + public function testEventFilenameFunctions(string $filename) { + $event = new FileUploadSanitizeNameEvent($filename, ''); + $this->assertSame($filename, $event->getFilename()); + } + + /** + * Provides data for testEventFilenameFunctions(). + * + * @return array + * Arrays with original file name. + */ + public function provideFilenames() { + return [ + 'ASCII filename with extension' => [ + 'example.txt', + ], + 'ASCII filename with complex extension' => [ + 'example.html.twig', + ], + 'ASCII filename with lots of dots' => [ + 'dotty.....txt', + ], + 'Unicode filename with extension' => [ + 'Ä Ö Ü Å Ø äöüåøhello.txt', + ], + 'Unicode filename without extension' => [ + 'Ä Ö Ü Å Ø äöüåøhello', + ], + ]; + } + + /** + * @covers ::stopPropagation + */ + public function testStopPropagation() { + $this->expectException(\RuntimeException::class); + $event = new FileUploadSanitizeNameEvent('test.txt', ''); + $event->stopPropagation(); + } + +}