Anonymous, managed_file form element and private file path

Created on 18 November 2022, over 2 years ago
Updated 9 February 2023, over 2 years ago

Problem/Motivation

Anonymous user are able to fill a form which contains a custom "managed_file" form element. This element uploads files in a private location for security reason.

i.e:

$form['files'] = [
    '#type' => 'managed_file',
    '#upload_location' => 'private://somewhere/',
    '#multiple' => TRUE,
];

Steps to reproduce

1. Create a custom form.
2. Add a file_manage form element which upload files into a private directory.
3. As an anonymous user, upload several files to the form.

Here comes the trouble. Even though the form will return the files (and files are well saved on the server), they are not available to the form anymore.

4. Try to delete one file using the removed selected button --> All files will be deleted.
5. (with or without 4.) Try submitting the form --> Files are not available in form_state.

Proposed resolution

I think we should move a bit down the access test in Drupal\file\Element\ManagedFile::valueCallback(), so we let temporary files being dealt with before we actually perform the access test.

Remaining tasks

Reviewing, testing.

🐛 Bug report
Status

Needs work

Version

9.5

Component
File system 

Last updated 3 days ago

Created by

🇭🇷Croatia Aporie

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇺🇸United States smustgrave

    As a bug it will need a test case to show the issue

    Thanks!

  • 🇭🇷Croatia Aporie

    I don't know, if I have some time I can try reproducing it on a fresh Drupal instance.

    If nobody can reproduce it, it might be a bug from the site I was working on.

  • 🇧🇪Belgium swentel

    Bitten by this one as well, the patch makes sense, and it works at the moment. Would be great to have some security eyes on this one!

  • 🇺🇸United States smustgrave

    Came up as a daily BSI triage target.

    Summary seems good. MR will have to be updated to 11.x and a test case added.

  • First commit to issue fork.
  • Merge request !12772Created MR. → (Open) created by mrinalini9
  • 🇮🇳India mrinalini9 New Delhi

    Created MR against 11.x.
    Still test needs to be added.

  • 🇭🇷Croatia Aporie

    Hi @smustgrave,

    Just spent a good afternoon trying to make a FunctionalJavascript test for it. Seems like a dead end. For some reason the ajax of the managed_file element is not working.

    I see that some kind of tweak was implemented in core/modules/file/tests/src/FunctionalJavascript/FileManagedFileElementTest.php to bypass it, so I tried going this way, adding $is_private to the form.

    Problem is, until the form won't get reloaded with ajax, then we can't reproduce the issue. The form_state never gets updated with empty $fids, and we can never reproduce the removal of all files when trying to remove only one.

    I'll see if I have some ideas coming up, but for now I feel beaten by this fracking ajax. Maybe we could test a Unit approach?

    If you have any idea, or pointers ... I can take another look at it next week.

Production build 0.71.5 2024