Commit 4e7959f8 authored by xjm's avatar xjm

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

(cherry picked from commit eae8a6c1)
parent 26918ae4
......@@ -2,6 +2,7 @@
namespace Drupal\Core\File;
use Drupal\Component\Utility\Unicode;
use Drupal\Core\File\Exception\DirectoryNotReadyException;
use Drupal\Core\File\Exception\FileException;
use Drupal\Core\File\Exception\FileExistsException;
......@@ -571,6 +572,10 @@ public function prepareDirectory(&$directory, $options = self::MODIFY_PERMISSION
* {@inheritdoc}
*/
public function getDestinationFilename($destination, $replace) {
$basename = $this->basename($destination);
if (!Unicode::validateUtf8($basename)) {
throw new FileException(sprintf("Invalid filename '%s'", $basename));
}
if (file_exists($destination)) {
switch ($replace) {
case FileSystemInterface::EXISTS_REPLACE:
......@@ -578,7 +583,6 @@ public function getDestinationFilename($destination, $replace) {
break;
case FileSystemInterface::EXISTS_RENAME:
$basename = $this->basename($destination);
$directory = $this->dirname($destination);
$destination = $this->createFilename($basename, $directory);
break;
......@@ -595,9 +599,13 @@ public function getDestinationFilename($destination, $replace) {
* {@inheritdoc}
*/
public function createFilename($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 FileException(sprintf("Invalid filename '%s'", $original));
}
if (substr(PHP_OS, 0, 3) == 'WIN') {
// These characters are not allowed in Windows filenames.
$basename = str_replace([':', '*', '?', '"', '<', '>', '|'], '_', $basename);
......
......@@ -439,6 +439,9 @@ public function prepareDirectory(&$directory, $options = self::MODIFY_PERMISSION
* @return string
* File path consisting of $directory and a unique filename based off
* of $basename.
*
* @throws \Drupal\Core\File\Exception\FileException
* Implementation may throw FileException or its subtype on failure.
*/
public function createFilename($basename, $directory);
......@@ -457,6 +460,9 @@ public function createFilename($basename, $directory);
* @return string|bool
* The destination filepath, or FALSE if the file already exists
* and FileSystemInterface::EXISTS_ERROR is specified.
*
* @throws \Drupal\Core\File\Exception\FileException
* Implementation may throw FileException or its subtype on failure.
*/
public function getDestinationFilename($destination, $replace);
......
......@@ -1038,7 +1038,13 @@ function _file_save_upload_single(\SplFileInfo $file_info, $form_field_name, $va
}
/** @var \Drupal\Core\File\FileSystemInterface $file_system */
$file_system = \Drupal::service('file_system');
try {
$file->destination = $file_system->getDestinationFilename($destination . $file->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.
if ($file->destination === FALSE) {
......
......@@ -2,7 +2,9 @@
namespace Drupal\Tests\file\Functional;
use Drupal\Component\Render\FormattableMarkup;
use Drupal\Core\File\FileSystemInterface;
use Drupal\Core\Url;
use Drupal\file\Entity\File;
use Drupal\Tests\TestFileCreationTrait;
......@@ -371,4 +373,60 @@ public function testDrupalMovingUploadedFileError() {
]), 'Found upload error log entry.');
}
/**
* Tests that filenames containing invalid UTF-8 are rejected.
*/
public function testInvalidUtf8FilenameUpload() {
$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' => FileSystemInterface::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,
],
],
'cookies' => $this->getSessionCookies(),
'http_errors' => FALSE,
];
$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);
}
}
......@@ -3,6 +3,7 @@
namespace Drupal\KernelTests\Core\File;
use Drupal\Component\PhpStorage\FileStorage;
use Drupal\Core\File\Exception\FileException;
use Drupal\Core\File\FileSystemInterface;
/**
......@@ -156,6 +157,10 @@ public function testFileDestination() {
$this->assertNotEqual($path, $destination, 'A new filepath destination is created when filepath destination already exists with FileSystemInterface::EXISTS_RENAME.', 'File');
$path = $file_system->getDestinationFilename($destination, FileSystemInterface::EXISTS_ERROR);
$this->assertEqual($path, FALSE, 'An error is returned when filepath destination already exists with FileSystemInterface::EXISTS_ERROR.', 'File');
// Invalid UTF-8 causes an exception.
$this->setExpectedException(FileException::class, "Invalid filename 'a\xFFtest\x80€.txt'");
$file_system->getDestinationFilename("core/misc/a\xFFtest\x80€.txt", FileSystemInterface::EXISTS_REPLACE);
}
/**
......
......@@ -2,6 +2,7 @@
namespace Drupal\Tests\Core\File;
use Drupal\Core\File\Exception\FileException;
use Drupal\Core\File\FileSystem;
use Drupal\Core\Site\Settings;
use Drupal\Tests\UnitTestCase;
......@@ -175,4 +176,16 @@ protected function assertFilePermissions($expected_mode, $uri, $message = '') {
$this->assertSame($expected_mode, $actual_mode, $message);
}
/**
* Tests that invalid UTF-8 results in an exception.
*
* @covers ::createFilename
*/
public function testInvalidUTF8() {
vfsStream::setup('dir');
$filename = "a\xFFsdf\x80€" . '.txt';
$this->setExpectedException(FileException::class, "Invalid filename '$filename'");
$this->fileSystem->createFilename($filename, 'vfs://dir');
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment