Problem/Motivation

This module has an Information Disclosure vulnerability.

Exposure of realpath() by \Drupal\encrypt_file\EncryptStreamWrapper through extension of \Drupal\Core\StreamWrapper\LocalStream combined with \Drupal\Core\File\FileSystem operations results in multiple execution flows where data may be stored at rest in unencrypted form or possibly be corrupted due to loss of cryptographic key association.

Steps to reproduce

Scenario 1

A \Drupal\encrypt\Exception\EncryptException in \Drupal\file_encrypt\StreamFilter\EncryptStreamFilter:23 can can results in FileSystem::move_uploaded_file() (uploading a file) utilizing a fallback to determine destination using realpath() bypassing the encrypt filter allowing the data to be stored on disk un-encrypted rather than presenting an error to the user.

Replication can occur by uploading a file through a field configured to store a file in the encrypt:// streamWrapper with the configured encryption profile being misconfiguration (such as using the wrong key length for a RealAES encryption module) or otherwise throws an exception.

Mitigating factors: The encryption profile must be experiencing error conditions.

Scenario 2

ImageStyle creations processes through \Drupal\system\Plugin\ImageToolkit\GDToolKit::save() calls FileSystem::move() which utilizes realpath() as initial attempt to rename files resulting in an unencrypted derivative being stored on disk.

Replication can be achieved by uploading an image file into an image field configured with an image style and visiting the generated image style link.

Mitigating factors: an attacker must be able to generate image styles for the source image.

Scenario 3

Moving(renameing) between encryption profiles will not result in the file being re-encrypted under the new profile due to LocalStream::rename() direct use of local file paths bypassing the the read/write filters of the encrypt:// wrapper.

# Assumes that profile1 and profile2 exist, and either use different keys or encryption algorithms.
file_put_contents('encrypt://profile1/test.txt', ‘test file’);
$raw_data_1 = file_get_contents(\Drupal::service('file_system')->realpath('encrypt://profile1/test.txt'));
\Drupal::service(‘file_system’)->move('encrypt://profile1/test.txt", 'encrypt://profile2/test2.txt');
$raw_data_2 = file_get_contents(\Drupal::service('file_system')->realpath('encrypt://profile2/test2.txt'));
assert($raw_data_1 !== $raw_data_2);

Code that latter attempts to read the file under profile2(believing it has been migrated) will receive corrupted data from file_get_contents() without ability to detect the corruption.

Related Links

https://drupal.slack.com/archives/C7UU7899C/p1735688179004509?thread_ts=...
https://www.drupal.org/project/file_encrypt/issues/3496795

Preliminary CVSS

CVSS:4.0/AV:N/AC:H/AT:P/PR:N/UI:N/VC:L/VI:L/VA:N/SC:N/SI:N/SA:N
CVSS v4.0 Score: 6.3 / Medium

Disclosure Timeline

January 1st: Incident reported to maintainers with notice report is subject to a 90 day disclosure timeline.
January 2nd: Maintainer Travis Carden (traviscarden) requested to be removed from the vulnerability report.
January 7th: Maintainer Alexander Shumenko (shumer) acknowledged the incident report.
January 21st: Contacted maintainers including @shumer to inquire if they had any questions about the vulnerability report.
March 6th: Contacted maintainers including @shumer requesting a status update.
March 18th: Provided maintainers including @shumber ~14 day notice of public disclosure in accorance with initial report.
April 1st: 90 day period has elapsed. Maintainers show now intent to resolve issue. Public Disclosure per disclosure timeline.

Disclosure Policy

Maintainers were advised of the following policy during initial notification:

This bug is subject to a 90 day disclosure policy. If a fix for this issue is made available to users before the end of the 90-day timeline, this bug report will become public 30 days after the fix was made available. Otherwise, this bug report will become public at the 90 day mark. The timeline may be shortened in response to an increase in public risk.

Maintainers included in initial report

Alexander Shumenko (shumer)
Travis Carden (traviscarden)
Rick Hawkins (rlhawk)
Adam Bergstein (nerdstein)
Daniel Wehner (dawehner)

Proposed resolution

Resolve vulnerable execution paths.

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork file_encrypt-3516802

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

cmlara created an issue. See original summary.

dmundra made their first commit to this issue’s fork.

dmundra’s picture

dmundra
he/him
Primary language English
commented
Assigned: Unassigned » dmundra

dmundra opened merge request !10

dmundra’s picture

dmundra
he/him
Primary language English
commented

I believe I tackled the first one by essentially not catching the exception and letting it be thrown all the way up. Not ideal but avoids the file being upload in an unencrypted status.

dmundra’s picture

dmundra
he/him
Primary language English
commented
Related issues: +#3215236: Image styles with imagemagick
dmundra’s picture

dmundra
he/him
Primary language English
commented

I am not sure that image styles works at all as the Image styles process cannot read the file since it is not decrypted for it. I think that was pointed out in related issues.

For image styles to work, we would have to either a process where the image is decrypted then styles are applied and then re-encrypted. This would be issue that scenario 2 is getting at I believe. I don't see how else image styles would work. To sufficiently not cause unencrypted data at rest it would better to not sure that feature and for it to be removed.

cmlara’s picture

cmlara
he/him
commented

For image styles to work, we would have to either a process where the image is decrypted then styles are applied and then re-encrypted.

To sufficiently not cause unencrypted data at rest it would better to not sure that feature and for it to be removed.

This is the purpose of streamWrappers, they handle this without 3rd party code needing to be aware this is occurring. Anything stored in the encrypt:// scheme should always be stored on disk encrypted.

I am not sure that image styles works at all as the Image styles process cannot read the file since it is not decrypted for it.

I no longer have the development lab for this report provisioned due to the incident age.

At this time I can not positively confirm that I did not accidentally test scenario 2 on a file experiencing the same faults as described in scenario 1. It is possible that this scenario has never seen a public exploit path. However the scenario 2 as noted in the issue would still exist if the file becomes readable and is separate from a reading fault.

I want to reiterate what was provided in the original security report and above, all 3 of these faults can be traced to the same root cause and should be 'same fix solution' to resolve. I would additionally not be surprised if fixing the root cause of this issue would resolve other non-security related bug reports.

dmundra’s picture

dmundra
he/him
Primary language English
commented

Thank you cmlara. I will try testing by removing the fix for scenario 1 and seeing if that makes scenario 2 happen.

dmundra’s picture

dmundra
he/him
Primary language English
commented

I merged some changes from other tickets and went back trying that and I finally got it to generate styles. Not sure why it started working (also updated local to 10.5). I do see it generating a styles image that is not encrypted. It doesn't work with the webp image styles but does for a simpler style like just cropping. However the image that is shown is still the regular image and not the cropped one. Need to continue to dig into the code more.