Issue #2461845 by Fabianx, Berdir, larowlan, David_Rothstein, Gábor Hojtsy: Private files that are no longer attached to an entity should not suddenly become accessible to people who couldn't see them before

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);
 

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.

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.');
   }
 }

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');
 }