diff --git a/core/modules/file/file.module b/core/modules/file/file.module index ee12a41fe2d..dea87de5b5a 100644 --- a/core/modules/file/file.module +++ b/core/modules/file/file.module @@ -591,6 +591,20 @@ function file_file_download($uri) { return; } + // Find out if a temporary file is still used in the system. + if ($file->isTemporary()) { + $usage = \Drupal::service('file.usage')->listUsage($file); + if (empty($usage) && $file->getOwnerId() != \Drupal::currentUser()->id()) { + // Deny access to temporary files without usage that are not owned by the + // same user. This prevents the security issue that a private file that + // was protected by field permissions becomes available after its usage + // was removed and before it is actually deleted from the file system. + // Modules that depend on this behavior should make the file permanent + // instead. + return -1; + } + } + // Find out which (if any) fields of this type contain the file. $references = file_get_file_references($file, NULL, EntityStorageInterface::FIELD_LOAD_CURRENT, NULL); diff --git a/core/modules/file/src/Tests/DownloadTest.php b/core/modules/file/src/Tests/DownloadTest.php index 22bee7a56de..811ea3334e3 100644 --- a/core/modules/file/src/Tests/DownloadTest.php +++ b/core/modules/file/src/Tests/DownloadTest.php @@ -60,6 +60,13 @@ class DownloadTest extends FileManagedTestBase { // Create a file. $contents = $this->randomMachineName(8); $file = $this->createFile(NULL, $contents, 'private'); + // Created private files without usage are by default not accessible + // for a user different from the owner, but createFile always uses uid 1 + // as the owner of the files. Therefore make it permanent to allow access + // if a module allows it. + $file->setPermanent(); + $file->save(); + $url = file_create_url($file->getFileUri()); // Set file_test access header to allow the download. diff --git a/core/modules/file/src/Tests/FilePrivateTest.php b/core/modules/file/src/Tests/FilePrivateTest.php index 996ffbb7b45..4a3a17d4eb7 100644 --- a/core/modules/file/src/Tests/FilePrivateTest.php +++ b/core/modules/file/src/Tests/FilePrivateTest.php @@ -45,6 +45,7 @@ class FilePrivateTest extends FileFieldTestBase { $test_file = $this->getTestFile('text'); $nid = $this->uploadNodeFile($test_file, $field_name, $type_name, TRUE, array('private' => TRUE)); \Drupal::entityManager()->getStorage('node')->resetCache(array($nid)); + /* @var \Drupal\node\NodeInterface $node */ $node = $node_storage->load($nid); $node_file = File::load($node->{$field_name}->target_id); // Ensure the file can be viewed. @@ -69,7 +70,8 @@ class FilePrivateTest extends FileFieldTestBase { $node_file = File::load($node->{$no_access_field_name}->target_id); // Ensure the file cannot be downloaded. - $this->drupalGet(file_create_url($node_file->getFileUri())); + $file_url = file_create_url($node_file->getFileUri()); + $this->drupalGet($file_url); $this->assertResponse(403, 'Confirmed that access is denied for the file without view field access permission.'); // Attempt to reuse the file when editing a node. @@ -94,5 +96,24 @@ class FilePrivateTest extends FileFieldTestBase { $this->assertTrue(empty($new_node), 'Node was not created.'); $this->assertUrl('node/add/' . $type_name); $this->assertRaw(SafeMarkup::format($constraint->message, array('%type' => 'file', '%id' => $node_file->id()))); + + // Now make file_test_file_download() return everything. + \Drupal::state()->set('file_test.allow_all', TRUE); + // Delete the node. + $node->delete(); + // Ensure the file can still be downloaded by the owner. + $this->drupalGet($file_url); + $this->assertResponse(200, 'Confirmed that the owner still has access to the temporary file.'); + + // Ensure the file cannot be downloaded by an anonymous user. + $this->drupalLogout(); + $this->drupalGet($file_url); + $this->assertResponse(403, 'Confirmed that access is denied for an anonymous user to the temporary file.'); + + // Ensure the file cannot be downloaded by another user. + $account = $this->drupalCreateUser(); + $this->drupalLogin($account); + $this->drupalGet($file_url); + $this->assertResponse(403, 'Confirmed that access is denied for another user to the temporary file.'); } } diff --git a/core/modules/file/tests/file_test/file_test.module b/core/modules/file/tests/file_test/file_test.module index 2c0b21ed389..e014246a6f9 100644 --- a/core/modules/file/tests/file_test/file_test.module +++ b/core/modules/file/tests/file_test/file_test.module @@ -150,6 +150,11 @@ function file_test_file_validate(File $file) { * Implements hook_file_download(). */ function file_test_file_download($uri) { + if (\Drupal::state()->get('file_test.allow_all', FALSE)) { + $files = entity_load_multiple_by_properties('file', array('uri' => $uri)); + $file = reset($files); + return file_get_content_headers($file); + } _file_test_log_call('download', array($uri)); return _file_test_get_return('download'); }