Problem/Motivation

Forgive me if this issue has been covered by some of the other issues linked to from #2821423: Dealing with unexpected file deletion due to incorrect file usage but I couldn't find this specific case listed anywhere.

The issue stems from the fact that file usage is only tracked on an entity's id, not the specific revision that a file is being used in. This can lead to instances where a file uploaded to the private file system and linked to via a text/text_long/text_with_summary field is still accessible in an unpublished revision.

Steps to reproduce:
1) Upload a file to a field that stores files in the private file system
2) Reference that file in a text field (like the body field on a node) (we'll call this Node A)
3) Delete the file from the field you previously uploaded it to
4) Create a new revision of Node A, deleting the link to the file in the text field and unpublishing the old revision (WBM or CM may be required here)
5) Make sure the new revision of Node A is published
6) Attempt to access the file by path directly

Expected:
File is inaccessible as it isn't referenced by a published revision.

Actual:
File is accessible due to editor's implementation of hook_file_download. This gathers usage via \Drupal::service('file.usage')->listUsage($file); and checks whether the user is able to view the entity. Since the entity is published, and it only loads the current revision, the user is able to access the file.

The use case for needing to deny access to these files comes back to what the private file system is usually used for: limiting access to files. If a file has been removed and a new version has been uploaded, but the previous version was once linked to in a text field then it should not be accessible.

Proposed resolution

TBD

Comments

acbramley created an issue. See original summary.

cilefen’s picture

cilefen commented

This seems almost like #2887988: Translation of file count is incorrect or #1239558: Deleting an entity with revisions and file/image field does not release file usage of non-default revisions, causing files to linger.

I advise adding a comment on #2821423: Dealing with unexpected file deletion due to incorrect file usage mentioning this issue 

[#3059494]

to get some eyes on it.

acbramley’s picture

acbramley commented

Thanks @cilefen will do!

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.9 was released on November 6 and is the final full bugfix release for the Drupal 8.7.x series. Drupal 8.7.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.8.0 on December 4, 2019. (Drupal 8.8.0-beta1 is available for testing.)

Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave commented
Issue tags: +Bug Smash Initiative

@acbramley wonder if those other issues addressed this?

acbramley’s picture

acbramley commented

Unfortunately not, I can still reproduce this on HEAD.

seth h’s picture

seth h commented

This issue has an inverse effect as well, namely that when a new file is uploaded to an unpublished revision on a previously published entity, that file is permanently access denied.

Example: A draft update to some node has updated the file, and the reviewing user wants to check the file contents before marking the revision as published.

This is caused by the fact that the published revision is loaded when loading file references in `file_get_file_references()`.

There is even a comment documenting this.

// Iterate over the field items to find the referenced file and field
// name. This will fail if the usage checked is in a non-current
// revision because field items are from the current
// revision.
// We also iterate over all translations because a file can be linked
// to a language other than the default.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
commented

This issue was temporarily un-published by the security team. After some discussion, we decided that having the files remain in the filesystem is something that can be discussed in public, so I am re-publishing this issue. See https://www.drupal.org/psa-2023-07-12.

prudloff’s picture

prudloff commented
Issue tags: +Security improvements