Not possible to allow uploading files with any file extension

I think that is by design. See #803926: File field shouldn't allow any file extension to be uploaded when the list of allowed extensions is left blank

recategorizing

Subscribing.

This issue is really so painful when the only people with access to the file upload box are trusted users.

So if not being able to upload all extension types was done for security reasons it's understandable. However if under the "allowed extensions" field in your content type you try to specify the list of the most common Text, Video & Audio file formats you won't even get to the end of that list because the text field doesn't allow more than 128 characters!

This needs to be fixed ASAP, because if I'm trying to allow my clients to upload files for me, various files keep on being rejected ("File Type Not Allowed" errors) purely because I can't fit enough extensions into the "allowed extensions" field.

I'm bumping this to a "major" issue, because it's something that really has to be figured out. Thanks for your help everybody.

I'm copying over a great post which should help support our case. This is comment #61 in the original #803926: File field shouldn't allow any file extension to be uploaded when the list of allowed extensions is left blank thread.

I don't mean to reopen a solved issue, but for me, and I'm sure many others, there are certain use cases in which an option to allow ALL extensions should be available if desired.

The current implementation is a problem for me for the following reasons:

Extension field is limited to 128 characters

Assuming all extensions are 3 chars + 1 space, that is at most 32 extensions maximum which may be whitelisted. Half of that number is taken up by common video file extensions alone. ie: "3g2 2gp asf asx avi flv mov mp4 mpg mpeg rm swf vob wmv" = 55 chars. If one intends to make a general file attachment field, then you will of course need to include a large number of document formats (MS Office, MS Office XML, Open Office, etc.), image formats, archives, etc. In any case, the hard limit is quickly reached in any attempt to "whitelist" all often-used, and safe, file extensions.

Lack of an option to allow all extensions (or use a black-list) implies that there are no Drupal use cases warranting such a feature.

My site is a single-user environment with no file-upload permissions for site visitors, therefore there is no need for filtering of upload extensions. I appreciate the developers improving Drupal security by preventing often-used security holes from being abused by users. However, as a site owner, I choose to use Drupal as a CMS not because it is the easiest to set up or because it "Just Works". I use it because of its adaptability and ease of customization.

Please note that I have no problem with a default configuration that prevents the upload of dangerous files such as php, cgi, pl, py, etc. I do, however, think that it isn't a good idea for developers to decide for all Drupal users whether or not to allow file extensions across-the-board. That is a decision that should be left up to the web administrators, ideally with some appropriate descriptions of the various options and their pros/cons, much like the public/private file upload system selection.

For your consideration.

Best regards,
Daniel

I among many others am also in need of this use-case scenario. A wildcard to allow all extensions or some other resolution is needed. I look forward to hearing some ideas soon. :)

The easiest fix would be to simply set a #maxlength property to something much higher than the default of 128, say 512. Not a great solution, but it should resolve the issue for 98% of users.

A complete helper module and allow all tags solution

This is a general request for feedback. This could be added to FileField UI Extras module.

To provide easy extension selection tags, like "<images>" and "<movies>" for example and the "<insecure-allow-all>" tags.

Possible tag list.

<images>

for standard Drupal images (gif,jpeg,png)

<images-all>

for any image type; gif,jpeg,png,psd,tiff,...

<movies>

for movies

<documents>

for text and document files

<sound-files>

for mp3s, etc

<multimedia>

<movies>, <sound-files> and <images-all>

<insecure-allow-all>

for passing an empty string, renaming only the hardcoded "php,pl,py,cgi,asp,js" if the flag "allow_insecure_uploads" is set to 0.

No interface, but have these set using variables:

variable_get('filefield_uiextras_extensions_images', 'gif jpg jpeg png')

And then it gets a bit nasty. The valid extension list would save extension tokens like __FILE_EXT_IMAGES__, and these need to be replaced before saving the uploaded file. A quick code scan showed that the following may be required:

Add an additional processor to managed_file to alter the file extension list sent to file.js (update $element['upload']['#attached']['js'][1]['data']['file']['elements']['#-' . xxx . '-upload'])).

Overwrite the value callback (file_managed_file_value) to hook into the extension list before the file is saved, and also file_field_widget_value() which directly calls file_managed_file_value().

Love feedback on this from someone that knows the Drupal 7 file handling before trying myself. It would be easier to have an alter hook on the file extension before validation, but I guess that this is an API change which would not be allowed in Drupal 7 at this late stage...

Alan! This sounds great! If only I knew enough coding to help. But I will support you mentally and with any testing necessary. :))

Thanks so much!

This is feature request and as such drupal 8.

marcingy, since Alan is suggesting to add this to the FileField UI Extras module (which I believe is a good idea) shouldn't we keep it at D7 and mark it as an issue relevant to that module from now on?

I shouldn't have had to do this.

Here's a module that changes this in Drupal 7 back to Drupal 6's behavior. It also lengthens the allowed extensions field from 128 characters to 2048, just in case you have a long list of allowed extensions.

I'd love to do a better job on this, but I have other projects, and won't be adding any new features to this module. If someone wants to co-maintain it (or even take ownership of it), please contact me and let me know. This isn't exactly a complicated module -- it's 8 lines of code.

Here's the link:

http://drupal.org/project/allow_all_file_extensions

BartK, Thanks so much for this module! Greatly appreciated!

Here is a more general solution which we could implement in core.
Make it work similar to blocks with radio options: Only the listed extensions (default); All extensions except those listed. I would also suggest increasing the character limit for the field.
If anyone +1's this idea then I don't mind tackling it.

Just for clarification here, since I was confused myself. In D6, one could go to example.com/admin/settings/uploads and set which file extensions are allowed to upload. In D7, the allowed file extensions are associated with the field type for each content type. For example, for the article content type, there is a field_image and the allowed extensions are set at example.com/admin/structure/types/manage/article/fields/field_image.

Hope that helps someone who might otherwise be confused about this.

Apologies for the multiple comments. Problems with the computer and apparently these comments cannot be deleted by the user.

I contend that this is not a feature request this is a bug report.

Additionally, this is a bug report for Drupal 7.

1. This field is limited to 128 chars means only 32 extensions can be allowed (comment #6). Was this by design? Sorry, but if so this was not a good design decision.

2. Since when Drupal in the business of enforcing which files are and are not dangerous? .EXE files are only dangerous if installed manually, on a Windows machine.

Perhaps technically this is a feature request, but right now it is not possible to give users an omnipurpose upload. I say we allow a box to allow upload of any file.

Try explaining to a client why they can't upload a .zip or .pps file on their site because you could only allow 32 file types and you didn't think that would be a filetype they would choose.

For a site managed by group of trusted users, allowing them to upload any file is an appropriate decision and gives them their adult ability to decide which files are or are not dangerous for themselves. After all, we allow people to enable the PHP input filter which can foobar everything.

Happy to create a patch, in fact I will need to in order to deliver my clients a product that does not attempt to think for them.

See the backport policy.

My module fixes both of these issues:

http://drupal.org/project/allow_all_file_extensions

If you tried it in the past and ran into an issue with validation, that issue has been fixed. Also, the size of the text field is now 2048 characters, in case you need to specify a large number of file types.

@BartK, thanks much. This does the trick.

Attached patch simply changes the maxlength on the Allowed file extensions field to 2048.

@25 See #14

While I initially thought the blocks-like approach outlined in #14 sounded good, the more I thought about it, the less I liked it. There are thousands of potential extensions that a site admin would need to track if they were to choose: "Allow anything except these..."

Sure, it would be nice to allow any extension, but do you really run into that a lot?

I'm not saying it's not a valid use case, I'm just trying to find the best way to implement it without opening a gaping security hole. There may not be a way...

Would "*" in that field work?

The "gaping security hole" already exists. A dev can just allow .php file or .exe files. ;)

If you want to start babysitting, why not check to disallow those, too?

My point is that it should be possible to configure Drupal to be insecure, but that Drupal should default to secure configurations and warn about insecure ones.

I'm all in favor of having this default closed, but it's unacceptable not to allow the administrator to override that. If I, as a web administrator, would like to allow myself to upload files of any type, Drupal ought trust me to be smart enough to make that decision. A strongly worded warning would be far better.

I'll give this a shot tomorrow.

Lets try to make tests not break this time.

(also renamed patch)

there is always a better way...

Full issue summary

Was #24 committed? This should be added as a workaround until a better solution is added IMHO.

A whitelist / blacklist approach looks ok, but most users would have difficulty deciding what are the files that should be excluded. AKA, how many users would know what extensions that their web server would execute... ASP, CF, PHP, PHP4, .... The blacklist as it currently stands is also deficient in this regards.

Replaced:
+const fileAllowExtensionsListed = 0;
+const fileAllowExtensionsNotListed = 1;

With:
+var fileAllowExtensionsListed = 0;
+var fileAllowExtensionsNotListed = 1;

As const is not supported by IE 6/7/8/9
Reference: https://developer.mozilla.org/en/JavaScript/Reference/Statements/const

Removing whitespace

Take 3

Setting to needs review

Sorry, but this isn't major. It's also a feature request. There is a workaround in contrib for people who need this functionality today (thanks, BartK!).

Extending the length of the file extensions field looks like a nice, uncontroversial change that could be handled in a separate issue.

The "gaping security hole" already exists. A dev can just allow .php file or .exe files. ;)

If you want to start babysitting, why not check to disallow those, too?

My point is that it should be possible to configure Drupal to be insecure, but that Drupal should default to secure configurations and warn about insecure ones.

but it's unacceptable not to allow the administrator to override that. If I, as a web administrator, would like to allow myself to upload files of any type, Drupal ought trust me to be smart enough to make that decision. A strongly worded warning would be far better.

Just want to throw in my $0.02 and say I wholly agree with these comments I've quoted.

I apologize for not having anything directly productive to add to the issue at this point, but I want to add to the tone of the conversation about security in general: Make it secure by default and establish good practices, but I think it is wrong to take away choice completely.

Not being able to upload a file of any type to a generic file field is a bug in my eyes, and I feel like looking at it otherwise indicates to me that you are making your opinion from a stance of how to restrict users and prevent problems, rather than enabling them and promoting possibility. Personally, I'd rather Drupal be my car than my seat belt.

Security guidelines are generalizations after all, and they don't apply to every situation. Others before me have already stated perfectly adequate use cases to prove it in this instance.

I'm not expecting to have expressed any pivotal rationale that will sway the world here, but if I've made known the attitude of one other Drupal user out in the world then I've accomplished my goal.

#45: disallowed-extensions-997900-045.patch queued for re-testing.

Re-roll.

+++ b/core/modules/file/file.field.incundefined
@@ -85,16 +85,21 @@ function file_field_instance_settings_form($field, $instance) {
+    ), ¶

trailing space

Thx. Still nr

fixed trailing space, and a missing space

Just to be clear, I want this feature too, but it can be dangerous! Imagine this sequence of events:

1. Someone uploads this file (apologies if it is incorrect):
   

   require "/includes/bootstrap.php";
   drupal_bootstrap(DRUPAL_BOOTSTRAP_FULL);
   $root = user_load(0);
   $root->pass = "whatever";
   user_save($root);
   

2. They then go to sites/default/files/foo.php
3. There! Someone now has changed the root password and has full control of your site.

And no one wants that to happen! Although this is nice, be careful who can create that content type!

I want this feature too, but it can be dangerous!

As discussed in this issue already, the default value is to be restrictive, it is up to administrators to lighten those restrictions as they choose. If they choose to allow PHP uploads, then we should expect that they've configured their HTTPd appropriately to block them executing. We should not force limitations on admins.

@JamesK I totally agree with you. I was just trying to clarify.

I tested this using an unlimited file field, excluding txt extension
1/ upload test.xml - OK
2/ upload test.php gives two warnings

1. For security reasons, your upload has been renamed to wdl7_1.php.txt.
2. The specified file wdl7_1.php.txt could not be uploaded. Files with the following extensions are not allowed: txt txt.

Reason: file_munge_filename is called with $extensions = 'txt' so the logic doesn't work anymore.

Same goes for file_save_upload

Changes in this patch:

1. file_munge_filename now can handle an extension list regardless of whether it is a whitelist or a blacklist by passing the $inclusion argument. This is an incompatible API change. If anyone has a better suggestion for handling this, please let me know. One possibility for still allowing this to backport is to handle the $extensions argument as either a string or an array containing both the extensions string and the inclusion flag. This seems to result in an overly complicated API, so I'm not sure about it.
2. Tests which call file_munge_filename now pass the correct $inclusion argument.
3. file_save_upload now correctly alters the extension list in the case when the extension list is a blacklist and a potentially dangerous script has been renamed
4. As a minor unrelated change, file_save_upload now appends a underscore (_) to the potentially dangerous script file extension to mimic the behaviour of file_munge_filename

Tagged as needs tests because we need coverage for using blacklists in all the functions that validate extensions. I'd like to get some agreement on this feature before spending the time writing them.

+++ b/core/includes/file.incundefined
@@ -1180,12 +1193,12 @@ function file_unmanaged_move($source, $destination = NULL, $replace = FILE_EXIST
-function file_munge_filename($filename, $extensions, $alerts = TRUE) {
+function file_munge_filename($filename, $extensions, $inclusion = FILE_ALLOW_EXTENSIONS_LISTED, $alerts = TRUE) {

isn't it easier to add the new parameter to the end?

I did the same test as in #59

1. upload php file, got renamed to php_.txt (not expected)
2. uploaded html file, all ok (OK)
3. uploaded .txt.xml file, got renamed to .txt_.xml (expected)
4. tried uploading a .txt file, error message (OK)

1/ I think we need some more information for the users on 1, so they now that php|pl|py|cgi|asp|js are always renamed
1/ Why do we need to add the .txt extension to those files, why not use .php_
3/ I expected this but is it desirable?

Edit: typos

I think we need some more information for the users on 1, so they now that php|pl|py|cgi|asp|js are always renamed

Well this isn't new behaviour to this issue. Files ending in php|pl|py|cgi|asp|js were always renamed before this patch. I just added the _ to make the behaviour the same as file_munge_filename (and thus make it compatible with file_unmunge_filename). If this is controversial, I'll split it into its own issue.

I created a new issue for 67: #1540008: Allowed file extensions don't mention that some extensions need an override in settings.php

This patch fixes broken tests.

Re: #64: It is definitely easier, but it makes for an awkward ordering of arguments.

#69, true but is doesn't need an API change. tests are passing locally

what to do with:
1/ Why do we need to add the .txt extension to those files, why not use .php_
3/ I expected this but is it desirable?

I don't know. We would need to dig out whatever issue that got committed in.

#72: This sounds like a job for git blame.

Why do we need to add the .txt extension to those files, why not use .php_

is always been like that and the same goes for

uploaded .txt.xml file, got renamed to .txt_.xml

, this is needed for apache (according to the comments)

So only thing left is the order of the parameters, if we add it to the end this can be backported easily, if not I guess we cannot backport without breaking contrib?

Well there is already a semi-solution in contrib for D7. I'm OK with this being D8-only. Edit: This actually should be backportable. Do you think the awkward argument ordering is very important?

I have no idea, depends on how many contrib modules call function file_munge_filename file_unmanaged_move directly, i'll try to find out

Lots! You should not rename / reorder this in D7

@Alan: you saw the eidt in #76, it's only file_munge_filename. If so any examples?

Sorry missed that. But for reference: Backport policy, particularly the points at the bottom.

[edit]
I have used this once on a Drupal 6 site. I can not think of any contrib modules that use it.

I would suggest, doing the D8 fix with the "awkward parameters", backport if the powers to be are happy with it, then introduce a separate D8 issue to clean this up. An API change usually means a much longer wait in the queues or (worst case) it never gets committed.

I found 3 contrib modules (so far) using file_munge_filename, I'll assume there're more, so @JamesK i think we should go with the awkward argument ordering

http://drupalcode.org/project/video.git?a=search&h=HEAD&st=grep&s=file_m...
http://drupalcode.org/project/services.git?a=search&h=HEAD&st=grep&s=fil...
http://drupalcode.org/project/flashvideo.git?a=search&h=HEAD&st=grep&s=f...

patch attached with the new order

Follow up created: #1540514: Re-order parameters of file_munge_filename

I added 2 new tests to test the exclusion of extensions as well.

Thanks attiks, this is great.

I removed some whitespace, and renamed the parameter in file_validate_extensions.

woops

remove white space from tests

tagging for (eventual) backport.
cross-linked to #803926: File field shouldn't allow any file extension to be uploaded when the list of allowed extensions is left blank

Fixing tags per http://drupal.org/node/1517250

#85: disallowed-extensions-997900-85.patch queued for re-testing.

I just want to throw in my opinion after wasting several hours of my time dealing with this, which is most certainly a bug in Drupal 7 and not just a new feature request. I have a client who expressly asked to be able to upload any file type on their website. For this client's website, that is a reasonable request. It doesn't create a security hole because only trusted personnel will have user accounts. Right now, Drupal 7 provides no way for me to accommodate that request. The allow_all_file_extensions module is helpful on other websites, but it doesn't work on the website I'm building, which uses the Media module and the file_entity module. After spending a few hours trying to understand how these various modules affect file extension validation, I've just given up and decided to simply hack the file_validate_extensions() function so it doesn't validate at all. I hate hacking Drupal core, but I also hate losing hours of my life trying to fix a problem that only exists because some over-officious people decided that they know better than me what my clients need or can be trusted to handle.

If someone cares to help me figure out how to meet my client's needs without hacking core, I'd love the advice. In the meantime, I just want to second the opinion of the guy who wrote that Drupal should "Make it secure by default and establish good practices, but I think it is wrong to take away choice completely." In fact, taking away choice completely is self-defeating, because it forces developers like me to adopt even worse practices such as hacking core, just so we can escape the prison that has been built for us.

#90: best way forward is to test the patch so it can be committed to D8 and backported to D7

@attiks: That sounds like a good suggestion. Unfortunately, I don't have time myself right now to test the patch in D8. I'll try to come back to this issue after I've met my client's immediate needs.

#86: i997900-86.patch queued for re-testing.

I didn't really expect it to still apply. Anyway, #91 looks like we need yet another reroll first.

remaining tasks

I took over all the changes from #86 and applied them onto the current code base, but I think I'm lacking a bit experience. Especially the file.js has quite changed. I commented out the relevant part in the JS file, as it doesn't make sense anymore. The file upload on a node form breaks at the moment with my patch applied.

Can somebody else (more experienced) take it over from here?

This looks to me like a dupe of #838456: Ability to allow any extension in file uploads

nm, the approach here is obviously better and more work has been done here. Allowing the swap to a blacklist instead of a whitelist for file extensions totally makes sense.

I'd like to say that I think if we're adding functionality to replace the file extension whitelist with a blacklist, we should have an equivalent "safe blacklist" that we provide by default containing known-unsafe file extensions (.exe, .php, etc..), otherwise the patch isn't much more palatable than #838456: Ability to allow any extension in file uploads.

rerolled against latest

not sure if this needs bumped to 8.1.x or not

Rerolled once more to fix a number of failing issues:
- form state is now a classed object
- swap FileInterface for File
- swap getFilename() for filename
- change "Allowed types" help text to "Disallowed types" as applicable

Hi, is there still interest in this? BTW, the patch does not apply cleanly to 8.4

I'm still very much interested in this.

Still using the patch in D7. If client would finally get around to upgrading, this would be a requirement.

Reroll of #108 against 8.7.x

Adding some relevant related issues:
#3021652: Deprecate upload-related functions file.inc and move to a file upload service will definitely conflict with this, since it's moving file_munge_filename() to a service.
#2492171: Provide options to sanitize filenames (transliterate, lowercase, replace whitespace, etc) will also conflict with this, since it's making some changes to how we rename files inside _file_save_upload_single()

Mostly just FYI. I have no idea what order or timeframe any of these patches might actually land in...

p.s. I feel partly responsible for this, since I helped make #803926: File field shouldn't allow any file extension to be uploaded when the list of allowed extensions is left blank happen in the first place. ;)

Thanks for the FYI, though not sure I have bandwidth to keep track of all those threads.

p.s. I feel partly responsible for this, since I helped make #803926: File field shouldn't allow any file extension to be uploaded when the list of allowed extensions is left blank happen in the first place. ;)

:shakes fists:

I used the patch in #119 in combination with a custom file upload widget, and I keep getting this warning, the built in drupal validation fails on file.js line 86, because the file extensions I had to provide in my custom widget are no longer a string, but an array of string and boolean value, as I have to set it inthe default settings of my custom widget, to get rid of PHP warnings:

 public static function defaultFieldSettings() {
    $settings = [
        'file_extensions' => [
          0 => implode(' ', self::$allowedExtensions),
          1 => FILE_ALLOW_EXTENSIONS_LISTED,
        ],
        'file_directory'  => 'video',
      ] + parent::defaultFieldSettings();

See screenshot for the warning:

Not sure if I did something wrong in my custom widget, I fixed it internally by patching core on my project, just wanted to point this out for further development of this issue.

Note that there are serious security concerns with doing this. It is completely against the recommendations of OWasp - see https://www.owasp.org/index.php/Unrestricted_File_Upload

+1000 to #126. I'd still love to "works as designed" this issue, instead of "fixing" it.

Got this from the bugsmash bingo.

I read the issue summary and then skimmed the comments. I do not see a bug here, this is more like a feature request. And It was pointed out that there are solutions in contrib. It is also informative that this issue opened in 2010 and was steadily discussed until May 2014, then it became very quiet here. Perhaps there is no longer interest in this change being in core? Most telling is the comment by alexpott in #126 referencing OWASP documentation explaining why something like this is not good practice. Therefor, I can't see this being made available in Drupal core.

So, changing to a feature request and closing as works as designed (per suggestion in #127).

As always, if you disagree, re-open the issue and comment on what still needs to be done.

Thanks!