Originally reported to the Drupal security team by @andrewbelcher on 18 April 2018 (#166363). Assuming it affects the latest version, this issue's version is set to D10.0.
---
(Original post)
Due to the information disclosure we've encountered, I'm raising this as a security issue, though I'm not 100% sure it counts in that category.
The issue
We have a system that users submit a claim for compensation. This collects a number of details, including an image (image field) upload which contains supporting evidence. This supporting evidence can contain personally identifiable information.
We have had a couple occurrences where a customer has reported that the image to their claim is not the image they uploaded as part of their submission.
Upon investigation, these claims (and a few additional unreported cases) point to file entities where the URI is also in use by another file entity. The uploaded image is appropriate for the other claim.
So it looks somewhere, either two separate form submissions are creating file entities pointing to the same physical file. My assumption is that the second file is overwriting the first, though I'm not 100% certain about this yet.
Although the issue has re-occurred, we have not yet been able to reproduce it ourselves.
Implementation details
We are on Drupal 8.4.6, though this issue goes back a couple of months, so covers multiple versions of 8.4.x.
This is on a custom view mode of a custom content entity, SQL storage. The form object has sizable customisation, including other AJAX elements that result in form rebuild etc.
The form is accessible and submitted by both anonymous users and authenticated users.
The image field is standard unlimited cardinality image base field:
BaseFieldDefinition::create('image')
->setLabel('Upload')
->setTargetEntityTypeId('file')
->setDescription('')
->setRequired(TRUE)
->setSettings([
'target_type' => 'file',
'alt_field' => FALSE,
'alt_field_required' => FALSE,
'uri_scheme' => 'private',
])
->setCardinality(FieldStorageDefinitionInterface::CARDINALITY_UNLIMITED)
->setDisplayOptions('view', [
'label' => 'above',
'type' => 'image',
'weight' => 12,
])
->setDisplayOptions('form', [
'type' => 'image_image',
'weight' => 5,
])
->setDisplayConfigurable('form', TRUE)
->setDisplayConfigurable('view', TRUE)
The form display uses the following config:
type: image_image
weight: 4
settings:
progress_indicator: throbber
preview_image_style: thumbnail
third_party_settings: { }
region: content
There is no customisation for the image upload on the form or from any form alters. Our theme adds some classes to the widget, but doesn't make any other adjustments.
Information from our investigation
An example in our file_managed table is:
310985 8914944b-99d9-4a52-81c9-349a13bdb054 en 0 image.jpg private://2018-03/image_5950.jpg image/jpeg 2304772 1 1522139633 1522139688
310999 f930313a-cbd5-4a6c-a840-7bd5dfebd94c en 0 image.jpg private://2018-03/image_5951.jpg image/jpeg 2192873 1 1522139649 1522139697
311126 3013d0a8-6a6b-4cea-add8-dea4c1cb23b0 en 0 image.jpg private://2018-03/image_5950.jpg image/jpeg 2975758 1 1522139766 1522139798
So looking at the first/last rows, it looks like two users have uploaded a file named image.jpg (although we haven't had confirmation from a customer that the images uploaded did have the same name) at similar times (133 seconds interval) and both files have ended up pointing to the same physical file 2018-03/image_5950.jpg.
I initially thought it might be a race condition as the first example I looked at had identical creation times, but that isn't always the case (as in this example and some with days between them) and there are files added in between with the same filename that get correctly enumerated.
I considered caching, but our form implements the page cache kill switch (though we do have the dynamic page cache) and I would have wouldn't expect caching issues to have a gap in-between instances.
The issue affects both anonymous and authenticated users, though I have not yet seen an example where a single file has has multiple users (counting all anon as one user).
The earliest conflict was with file entities created on 2018-02-01 and 2018-02-20, with the last both on 2018-03-27 (the above example). I can't see any changes in our commit logs are related to either core or this particular form that give any indication as to why it started then, so I am assuming it is down to an increase in submission volume revealing the issue.
I have looked through the image widget and managed file element code, including tracking back to file_managed_file_save_upload()/file_save_upload()/file_destination(). While it looks conceivable that there could be a race condition between file_create_filename() and </code>drupal_move_uploaded_file() in file_save_upload(), the times of our examples indicate that this is not our problem. I cannot see anything else that looks like a candidate for causing our problem...
I cannot see anything in our logs (apache or watchdog) that sheds any light on the situation...
I'm now at a bit of a loss for where I go next in attempting to debug/reproduce/problem solve the issue - any suggestions would be very welcome!
Problem/Motivation
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Contributors
- andrewbelcher
- effulgentsia
- David Strauss
- mcdruid
- catch
Coordination:
- xjm
- cliefen