diff --git a/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php b/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php index f84ae1d2ef..bf4e056ad5 100644 --- a/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php +++ b/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php @@ -180,8 +180,12 @@ public function post(Request $request, $entity_type_id, $bundle, $field_name) { throw new HttpException(500, 'Destination file path is not writable'); } + $validators = $this->getUploadValidators($field_definition); + + $prepared_filename = $this->prepareFilename($filename, $validators); + // Create the file. - $file_uri = "{$destination}/{$filename}"; + $file_uri = "{$destination}/{$prepared_filename}"; $temp_file_path = $this->streamUploadData(); @@ -198,8 +202,8 @@ public function post(Request $request, $entity_type_id, $bundle, $field_name) { // Begin building file entity. $values = [ 'uid' => $this->currentUser->id(), - 'filename' => $filename, - 'filemime' => $this->mimeTypeGuesser->guess($filename), + 'filename' => $prepared_filename, + 'filemime' => $this->mimeTypeGuesser->guess($prepared_filename), 'uri' => $file_uri, // Set the size. This is done in File::preSave() but we validate the file // before it is saved. @@ -209,7 +213,7 @@ public function post(Request $request, $entity_type_id, $bundle, $field_name) { // Validate the file entity against entity level validation and field level // validators. - $this->validate($file, $field_definition); + $this->validate($file, $field_definition, $validators); // Move the file to the correct location after validation. Use // FILE_EXISTS_ERROR as the file location has already been determined above @@ -307,6 +311,11 @@ protected function validateAndParseContentDispositionHeader(Request $request) { throw new BadRequestHttpException('No filename found in "Content-Disposition" header. A file name in the format "filename=FILENAME" must be provided'); } + // Check for the "filename*" format. This is currently unsupported. + if (!empty($matches['star'])) { + throw new BadRequestHttpException('The extended "filename*" format is currently not supported in the "Content-Disposition header"'); + } + // Don't validate the actual filename here, that will be done by the upload // validators in validate(). // @see file_validate() @@ -355,14 +364,16 @@ protected function validateAndLoadFieldDefinition($entity_type_id, $bundle, $fie * The file entity to validate. * @param \Drupal\Core\Field\FieldDefinitionInterface $field_definition * The field definition to validate against. + * @param array $validators + * AN array of upload validators to pass to file_validate(). * * @throws \Symfony\Component\HttpKernel\Exception\UnprocessableEntityHttpException */ - protected function validate(FileInterface $file, FieldDefinitionInterface $field_definition) { + protected function validate(FileInterface $file, FieldDefinitionInterface $field_definition, array $validators) { $this->resourceValidate($file); // Validate the file based on the field definition configuration. - $errors = file_validate($file, $this->getUploadValidators($field_definition)); + $errors = file_validate($file, $validators); if (!empty($errors)) { $message = "Unprocessable Entity: file validation failed.\n"; @@ -372,6 +383,46 @@ protected function validate(FileInterface $file, FieldDefinitionInterface $field } } + /** + * Prepares the filename to strip out any malicious extensions. + * + * @param string $filename + * The file name. + * @param array $validators + * The array of upload validators. + * + * @return string + * The prepared/munged filename. + */ + protected function prepareFilename($filename, array &$validators) { + // Build the list of non-munged extensions if the caller provided them. + $extensions = $validators['file_validate_extensions'][0]; + + + if (!empty($extensions)) { + // Munge the filename to protect against possible malicious extension + // hiding within an unknown file type (ie: filename.html.foo). + $filename = file_munge_filename($filename, $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). Don't rename if 'allow_insecure_uploads' + // evaluates to TRUE. + if (!\Drupal::config('system.file')->get('allow_insecure_uploads') && preg_match('/\.(php|pl|py|cgi|asp|js)(\.|$)/i', $filename) && (substr($filename, -4) != '.txt')) { + // The destination filename will also later be used to create the URI. + $filename .= '.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'; + } + } + + return $filename; + } + /** * Determines the URI for a file field. * diff --git a/core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php b/core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php index 59f4b226c1..e026e98191 100644 --- a/core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php @@ -150,44 +150,18 @@ public function testPostFileUpload() { // Check the actual file data. $this->assertSame($this->testFileData, file_get_contents('public://foobar/example.txt')); - // Test the file again but using 'filename*' in the Content-Disposition - // header. - $response = $this->fileRequest($uri, $this->testFileData, ['Content-Disposition' => 'file; filename*="example.txt"']); - - $this->assertSame(201, $response->getStatusCode()); - - $expected = $this->getExpectedNormalizedEntity(2, 'example_0.txt'); - - $this->assertResponseData($expected, $response); - - // Check the actual file data. - $this->assertSame($this->testFileData, file_get_contents('public://foobar/example_0.txt')); - // Test the file again but using 'filename' in the Content-Disposition // header with no 'file' prefix. $response = $this->fileRequest($uri, $this->testFileData, ['Content-Disposition' => 'filename="example.txt"']); $this->assertSame(201, $response->getStatusCode()); - $expected = $this->getExpectedNormalizedEntity(3, 'example_1.txt'); - - $this->assertResponseData($expected, $response); - - // Check the actual file data. - $this->assertSame($this->testFileData, file_get_contents('public://foobar/example_1.txt')); - - // Test the file again but using 'filename*' in the Content-Disposition - // header with no 'file' prefix. - $response = $this->fileRequest($uri, $this->testFileData, ['Content-Disposition' => 'filename*="example.txt"']); - - $this->assertSame(201, $response->getStatusCode()); - - $expected = $this->getExpectedNormalizedEntity(4, 'example_2.txt'); + $expected = $this->getExpectedNormalizedEntity(2, 'example_0.txt'); $this->assertResponseData($expected, $response); // Check the actual file data. - $this->assertSame($this->testFileData, file_get_contents('public://foobar/example_2.txt')); + $this->assertSame($this->testFileData, file_get_contents('public://foobar/example_0.txt')); } /** @@ -215,24 +189,19 @@ public function testPostFileUploadInvalidHeaders() { $response = $this->fileRequest($uri, $this->testFileData, ['Content-Disposition' => 'file; filename=""']); $this->assertResourceErrorResponse(400, 'No filename found in "Content-Disposition" header. A file name in the format "filename=FILENAME" must be provided', $response); - $response = $this->fileRequest($uri, $this->testFileData, ['Content-Disposition' => 'file; filename*=""']); - $this->assertResourceErrorResponse(400, 'No filename found in "Content-Disposition" header. A file name in the format "filename=FILENAME" must be provided', $response); - // An empty filename without a context in the Content-Disposition header // should return a 400. $response = $this->fileRequest($uri, $this->testFileData, ['Content-Disposition' => 'filename=""']); $this->assertResourceErrorResponse(400, 'No filename found in "Content-Disposition" header. A file name in the format "filename=FILENAME" must be provided', $response); - $response = $this->fileRequest($uri, $this->testFileData, ['Content-Disposition' => 'filename*=""']); - $this->assertResourceErrorResponse(400, 'No filename found in "Content-Disposition" header. A file name in the format "filename=FILENAME" must be provided', $response); - // An invalid key-value pair in the Content-Disposition header should return // a 400. $response = $this->fileRequest($uri, $this->testFileData, ['Content-Disposition' => 'not_a_filename="example.txt"']); $this->assertResourceErrorResponse(400, 'No filename found in "Content-Disposition" header. A file name in the format "filename=FILENAME" must be provided', $response); - $response = $this->fileRequest($uri, $this->testFileData, ['Content-Disposition' => 'not_a_filename*="example.txt"']); - $this->assertResourceErrorResponse(400, 'No filename found in "Content-Disposition" header. A file name in the format "filename=FILENAME" must be provided', $response); + // Using filename* extended format is not currently supported. + $response = $this->fileRequest($uri, $this->testFileData, ['Content-Disposition' => 'filename*="UTF-8 \' \' example.txt"']); + $this->assertResourceErrorResponse(400, 'The extended "filename*" format is currently not supported in the "Content-Disposition header"', $response); } /** @@ -394,6 +363,63 @@ public function testFileUploadLargerFileSize() { $this->assertFalse(file_exists('public://foobar/example.txt')); } + /** + * Tests using the file upload POST route with malicious extensions. + */ + public function testFileUploadMaliciousExtension() { + $this->initAuthentication(); + + $this->provisionResource([static::$format], static::$auth ? [static::$auth] : [], ['POST']); + + $this->setUpAuthorization('POST'); + + $uri = Url::fromUri('base:' . static::$postUri); + + $php_string = ''; + + // Test using a masked exploit file. + $response = $this->fileRequest($uri, $php_string, ['Content-Disposition' => 'filename="example.php.txt"']); + + $expected = $this->getExpectedNormalizedEntity(1, 'example.php_.txt', TRUE); + // Override the expected filesize. + $expected['filesize'][0]['value'] = strlen($php_string); + + $this->assertResponseData($expected, $response); + + $this->assertTrue(file_exists('public://foobar/example.php_.txt')); + + // Add php as an allowed format. Allow insecure uploads still being FALSE + // should still not allow this. So it should still have a .txt extension + // appended. + $this->field->setSetting('file_extensions', 'txt php') + ->save(); + $this->refreshTestStateAfterRestConfigChange(); + + $response = $this->fileRequest($uri, $php_string, ['Content-Disposition' => 'filename="example.php"']); + + $expected = $this->getExpectedNormalizedEntity(2, 'example.php.txt', TRUE); + // Override the expected filesize. + $expected['filesize'][0]['value'] = strlen($php_string); + + $this->assertResponseData($expected, $response); + + // Now allow insecure uploads. + \Drupal::configFactory() + ->getEditable('system.file') + ->set('allow_insecure_uploads', TRUE) + ->save(); + + $response = $this->fileRequest($uri, $php_string, ['Content-Disposition' => 'filename="example.php"']); + + $expected = $this->getExpectedNormalizedEntity(3, 'example.php', TRUE); + // Override the expected filesize. + $expected['filesize'][0]['value'] = strlen($php_string); + // The file mime should also now be PHP. + $expected['filemime'][0]['value'] = 'application/x-httpd-php'; + + $this->assertResponseData($expected, $response); + } + /** * {@inheritdoc} */