Hi there,

After updating from FE 2.2 to 2.3 I noticed that the default upload scheme is ignored when 'skip scheme selection' is selected in the module configuration.

This means that when the default scheme is set to private:// files get uploaded to the public file directory without notifying the user in any way. This seems like a pretty serious security issue to me!

*I think this issue may have to do with https://www.drupal.org/node/2264145 that was committed to FE 2.3, but I am not sure.

Steps to reproduce:

1. make sure you have set the default upload scheme to private:// at 'admin/config/media/file-system'.
2. upload a file at 'file/add'
3. check the file destination at 'file/xxx/edit' , the destination is now set to 'Public files' while it should be set to 'Private files'.

I added screenshots that should clarify the steps to reproduce above.

It should be noted that the behaviour described above does not occur when 'skip scheme selection' is not selected at the module configuration page. When acceptable this could serve as a workaround for the problem, but in cases where this is simply not acceptable from a UI perspective this remains a problem to be solved.

Especially the silent nature of this behaviour creates a security threat in my view. Users expect files to be uploaded to the private destination while files are uploaded to the public destination without any notification...

Cheers,
Marco

CommentFileSizeAuthor
#5 file_entity-scheme-selection-2905317-5.patch430 bytescboyden
Screen Shot 2017-07-10 at 17.32.42.png71.89 KBfrontmobe
Screen Shot 2017-07-10 at 16.57.38.png98.95 KBfrontmobe
Screen Shot 2017-07-10 at 16.55.40.png152.16 KBfrontmobe

Comments

Frontmobe created an issue. See original summary.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Related issues: +#2893574: Default upload scheme ignored when ‘skip scheme selection’ is selected
cboyden’s picture

cboyden commented
Related issues: +#1901566: Resolve the 'allowed file types' field widget setting with the file_entity forms

Looks like this was caused by http://cgit.drupalcode.org/file_entity/commit/?id=fe30d62d6e64749cccb291..., which was a fix for #1901566: Resolve the 'allowed file types' field widget setting with the file_entity forms.

cboyden’s picture

cboyden commented

See comment 18 in that issue - that's where code was added to set the form data to have uri_scheme = "public," to avoid PHP notices in cases various settings options weren't available. But obviously this means that in any case where the choose-destination step is skipped, file uploads will be public. It might work to instead grab the default scheme and use that. Or, if the approach of initializing the form options array is problematic in other ways too, it would require more checks in the code.

cboyden’s picture

cboyden commented
Version: 7.x-2.3 » 7.x-2.x-dev
Status: Active » Needs review
StatusFileSize
new file_entity-scheme-selection-2905317-5.patch430 bytes

The attached patch fixes the immediate problem. But, it's possible that the other generic form options being set will cause other problems. Perhaps the approach needs to be different.

  $form['#options'] = $options + array(
    'types' => array(),
    'enabledPlugins' => array(),
    'schemes' => array(),
    'max_filesize' => '',
    // Changed only this line below.
    'uri_scheme' => file_default_scheme(),
    'plugins' => ''
  );

Which other options in this array could cause problems? Does the code overwrite options that have already been set in the $options array?

  • cboyden authored 247afc5 on 7.x-2.x 
    Issue #2905317 by cboyden, Frontmobe: Default upload scheme ignored when...

  • cboyden authored 247afc5 on 7.x-3.x 
    Issue #2905317 by cboyden, Frontmobe: Default upload scheme ignored when...
joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Status: Needs review » Fixed

Committed cboyden patch as it looks good. If necessary, please open a new issue for related work, linking this to the new issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.