SA-CORE-2019-004 by alexpott, larowlan, greggles, drumm, mlhess, David_Rothstein, pwolanin

core/includes/file.inc

@@ -588,8 +588,15 @@ function file_build_uri($path) {
  * @return
  *   The destination filepath, or FALSE if the file already exists
  *   and FILE_EXISTS_ERROR is specified.
+ *
+ * @throws \RuntimeException
+ *   Thrown if the filename contains invalid UTF-8.
  */
 function file_destination($destination, $replace) {
+  $basename = drupal_basename($destination);
+  if (!Unicode::validateUtf8($basename)) {
+    throw new \RuntimeException(sprintf("Invalid filename '%s'", $basename));
+  }
   if (file_exists($destination)) {
     switch ($replace) {
       case FILE_EXISTS_REPLACE:
@@ -597,7 +604,6 @@ function file_destination($destination, $replace) {
         break;
 
       case FILE_EXISTS_RENAME:
-        $basename = drupal_basename($destination);
         $directory = drupal_dirname($destination);
         $destination = file_create_filename($basename, $directory);
         break;
@@ -769,11 +775,20 @@ function file_unmunge_filename($filename) {
  * @return
  *   File path consisting of $directory and a unique filename based off
  *   of $basename.
+ *
+ * @throws \RuntimeException
+ *   Thrown if the $basename is not valid UTF-8 or another error occurs
+ *   stripping control characters.
  */
 function file_create_filename($basename, $directory) {
+  $original = $basename;
   // Strip control characters (ASCII value < 32). Though these are allowed in
   // some filesystems, not many applications handle them well.
   $basename = preg_replace('/[\x00-\x1F]/u', '_', $basename);
+  if (preg_last_error() !== PREG_NO_ERROR) {
+    throw new \RuntimeException(sprintf("Invalid filename '%s'", $original));
+  }
+
   if (substr(PHP_OS, 0, 3) == 'WIN') {
     // These characters are not allowed in Windows filenames
     $basename = str_replace([':', '*', '?', '"', '<', '>', '|'], '_', $basename);

core/modules/file/file.module

@@ -984,7 +984,14 @@ function file_save_upload($form_field_name, $validators = [], $destination = FAL
     if (substr($destination, -1) != '/') {
       $destination .= '/';
     }
-    $file->destination = file_destination($destination . $file->getFilename(), $replace);
+    try {
+      $file->destination = file_destination($destination . $file->getFilename(), $replace);
+    }
+    catch (\RuntimeException $e) {
+      \Drupal::messenger()->addError(t('The file %filename could not be uploaded because the name is invalid.', ['%filename' => $file->getFilename()]));
+      $files[$i] = FALSE;
+      continue;
+    }
     // If file_destination() returns FALSE then $replace === FILE_EXISTS_ERROR and
     // there's an existing file so we need to bail.
     if ($file->destination === FALSE) {

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

+<?php
+
+namespace Drupal\Tests\file\Functional;
+
+use Drupal\Component\Render\FormattableMarkup;
+use Drupal\Core\Url;
+use GuzzleHttp\Cookie\CookieJar;
+
+/**
+ * Tests the file_save_upload() function.
+ *
+ * @group file
+ */
+class SaveUploadTest extends FileManagedTestBase {
+
+  /**
+   * Tests that filenames containing invalid UTF-8 are rejected.
+   */
+  public function testInvalidUtf8FilenameUpload() {
+    $account = $this->drupalCreateUser(['access site reports']);
+    $this->drupalLogin($account);
+    $this->drupalGet('file-test/upload');
+
+    // Filename containing invalid UTF-8.
+    $filename = "x\xc0xx.gif";
+
+    $page = $this->getSession()->getPage();
+    $data = [
+      'multipart' => [
+        [
+          'name'     => 'file_test_replace',
+          'contents' => FILE_EXISTS_RENAME,
+        ],
+        [
+          'name' => 'form_id',
+          'contents' => '_file_test_form',
+        ],
+        [
+          'name' => 'form_build_id',
+          'contents' => $page->find('hidden_field_selector', ['hidden_field', 'form_build_id'])->getAttribute('value'),
+        ],
+        [
+          'name' => 'form_token',
+          'contents' => $page->find('hidden_field_selector', ['hidden_field', 'form_token'])->getAttribute('value'),
+        ],
+        [
+          'name' => 'op',
+          'contents' => 'Submit',
+        ],
+        [
+          'name'     => 'files[file_test_upload]',
+          'contents' => 'test content',
+          'filename' => $filename,
+        ],
+      ],
+      'http_errors' => FALSE,
+    ];
+
+    $domain = parse_url($this->getUrl(), PHP_URL_HOST);
+    $session_id = $this->getSession()->getCookie($this->getSessionName());
+    $data['cookies'] = CookieJar::fromArray([$this->getSessionName() => $session_id], $domain);
+
+    $this->assertFileNotExists('temporary://' . $filename);
+    // Use Guzzle's HTTP client directly so we can POST files without having to
+    // write them to disk. Not all filesystem support writing files with invalid
+    // UTF-8 filenames.
+    $response = $this->getHttpClient()->request('POST', Url::fromUri('base:file-test/upload')->setAbsolute()->toString(), $data);
+
+    $content = (string) $response->getBody();
+    $this->htmlOutput($content);
+    $error_text = new FormattableMarkup('The file %filename could not be uploaded because the name is invalid.', ['%filename' => $filename]);
+    $this->assertContains((string) $error_text, $content);
+    $this->assertContains('Epic upload FAIL!', $content);
+    $this->assertFileNotExists('temporary://' . $filename);
+  }
+
+}

core/tests/Drupal/KernelTests/Core/File/DirectoryTest.php

@@ -147,6 +147,10 @@ public function testFileDestination() {
     $this->assertNotEqual($path, $destination, 'A new filepath destination is created when filepath destination already exists with FILE_EXISTS_RENAME.', 'File');
     $path = file_destination($destination, FILE_EXISTS_ERROR);
     $this->assertEqual($path, FALSE, 'An error is returned when filepath destination already exists with FILE_EXISTS_ERROR.', 'File');
+
+    // Invalid UTF-8 causes an exception.
+    $this->setExpectedException(\RuntimeException::class, "Invalid filename 'a\xFFtest\x80€.txt'");
+    file_destination("core/misc/a\xFFtest\x80€.txt", FILE_EXISTS_REPLACE);
   }
 
   /**

core/tests/Drupal/KernelTests/Core/File/FileCreateFilenameTest.php

+<?php
+
+namespace Drupal\KernelTests\Core\File;
+
+/**
+ * Tests file_create_filename().
+ *
+ * @group File
+ */
+class FileCreateFilenameTest extends FileTestBase {
+
+  /**
+   * Tests that invalid UTF-8 does not break file_create_filename().
+   */
+  public function testInvalidUTF8() {
+    $filename = "a\xFFsdf\x80€" . '.txt';
+    $this->setExpectedException(\RuntimeException::class, "Invalid filename '$filename'");
+    file_create_filename($filename, $this->siteDirectory);
+  }
+
+}