2 captures
23 Oct 2015 - 11 May 2024
Sep OCT Nov
23
2014 2015 2016
success
fail

About this capture

COLLECTED BY

Organization: Internet Archive

The Internet Archive discovers and captures web pages through many different web crawls. At any given time several distinct crawls are running, some for months, and some every day or longer. View the web archive through the Wayback Machine.

Collection: Wide Crawl Number 13

Web Wide Crawl Number 13
TIMESTAMPS

The Wayback Machine - http://web.archive.org/web/20151023062402/https://www.drupal.org/node/1848498
 
Skip to main content  Skip to search  









Get Started

Community

Documentation

Support

Download & Extend

Jobs

Marketplace

About

 








Drupal
 






Search form







Drupal Homepage

Log in / Register

 








Support the project you love. Become a Member Today 






Download & Extend
 




Download & Extend Home

Drupal Core

Distributions

Modules

Themes

 

You are here

Feeds » Issues

Respect allowed file extensions in file mapper.

 







Needs work
Project: 
Feeds
Version: 
6.x-1.x-dev
Component: 
Code
Priority: 
Critical
Category: 
Bug report
Assigned: 
twistor
Reporter: 
roeneman
Created: 
November 23, 2012 - 10:04
Updated: 
June 17, 2015 - 04:56


Log inorregister to update this issue
 



Jump to:

Most recent comment

Most recent attachment

 















Hi, I've an issue with mapping to file fields.

When importing emails through mailhandler, including attachments, the allowed file extensions of my file field are not respected.
This is actually a security issue for me, as all file types are now saved from email attachments, including php files, or whatever (malicious code) people want to send me.

My setup is mailhandler, with the Mailhandler IMAP stream parser, that maps the email 'attachments' to a field of type 'file'. It works great, I just need to filter out unwanted file types.

Can anyone help me with this? Is it actually Feeds, or perhaps an issue that should be solved through Mailhandler? Any help is really appreciated!

Thanks,
Roeneman
Files: 
CommentFileSizeAuthor
#20 core-adapted-security.patch11.36 KBBevan
#20 SDO120198-standalone-feeds.patch11.36 KBBevan
#17 feeds-1848498-17.patch10.81 KBtwistor
PASSED: [[SimpleTest]]: [MySQL] 7,214 pass(es).
[ View ]
#17 interdiff.txt501 bytestwistor
#15 feeds-1848498-15.patch10.81 KBtwistor
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in sites/default/modules/feeds/plugins/FeedsParser.inc.
[ View ]
#14 feeds-1848498-14.patch11.93 KBtwistor
PASSED: [[SimpleTest]]: [MySQL] 4,455 pass(es).
[ View ]
#11 feeds-1848498-11.patch10.29 KBtwistor
FAILED: [[SimpleTest]]: [MySQL] 4,447 pass(es), 5 fail(s), and 11 exception(s).
[ View ]

 

Comments

 
kerasai’s picture


 kerasai CreditAttribution: kerasai commented at 5:47pm 


Hello, the file type restrictions are imposed by the core Fields module and only affect content that is manipulated through the interface (node add/edit form). Unfortunately I don't know of a drop-in solution to your problem, but to do something like this you'd probably need to invoke hook_node_presave() to analyze the node's field data and unset fields with unacceptable file types.
 

Log inorregister to post comments


 roeneman’s picture


 roeneman CreditAttribution: roeneman commented 3 at 5:18pm 


@kerasai, thanks, this small pointer helped.

For others, for my use case the following solution worked.

Making use of hook_feeds_presave, I constructed the following (not an expert solution I'm sure, but it works for me):

<?php
hook_feeds_presave{
    //FILE EXTENSION CHECK
    If (isset($entity->field_attachments['und'])){
    //Make list of extensions seperated by a space.
    $extensions = 'rtf doc txt pdf jpg gif png bmp doc docx rtf ppt pptx xls xlsx';
    //Regex adopted from : http://api.drupal.org/api/drupal/includes!file.inc/function/file_validate_extensions/7
    $regex = '/\.(' . preg_replace('/ +/', '|', preg_quote($extensions)) . ')$/i';
    //Loop through array of file field, if match unset from array and delete file
    foreach ($entity->field_attachments['und'] as $k => $v){
        if (!preg_match($regex, $v['filename'])) {
            $file = file_load($v['fid']);
            unset($entity->field_attachments['und'][$k]);
            file_delete($file, $force = FALSE);
         }
     }
    }
?>


Hope this helps for someone.
Rgds, Roeneman
 

Log inorregister to post comments


 kerasai’s picture


 kerasai CreditAttribution: kerasai commented at 8:56pm 


Glad you got this going.

To make it simpler, I think you could pass each field value directly to file_validate_extensions and analyze it's return code to determine whether or not to unset it. Might be slightly more complicated than that, but no need to maintain the same code in 2 places.
 

Log inorregister to post comments


 roeneman’s picture


 roeneman CreditAttribution: roeneman commented 3 at 7:58am 


Project:Feeds» Mailhandler
Version:7.x-2.0-alpha7» 7.x-2.x-dev
Component:Code» Mailhandler

Moving this request to Mailhandler. In my opinion we should catch the files even before they are (temporary) saved.

So request is for a 'file exentions field' in mailhandler, with implementation of 'file_validate_extensions' function, removal of unwanted files from the import, and a (configurable) text in the mailbody (or creation of a .txt file) stating that the file has been removed. 

My implementation in hook_feeds_presave for adding text to the mailbody:
<?php
$entity->body["und"][0]["value"] = $entity->body["und"][0]["value"] ."\n" .'Attachment ' .$v['filename'] .' was removed, only the following extensions are allowed: ' .$extensions;
?>


I know that proposing a patch is the best way to do this, but that is (still) outside of my programming capabilities. Sorry for that. Please consider this request, as importing files with certain extensions is a safety risk!

Rgds, Roeneman
 

Log inorregister to post comments


 roeneman’s picture


 roeneman CreditAttribution: roeneman commented 3 at 8:01am 


@kerasai, indeed you're right. And additionally, when going for this feed_presave implementation, we should first fetch allowed extensions from the $entity->type file-field. This just as a note for someone wanting to go this direction, I believe this is something best implemented in Mailhandler.
 

Log inorregister to post comments


 kerasai’s picture


 kerasai CreditAttribution: kerasai commented at 5:01pm 


@roeneman good thinking.

I can't promise when, but I'll try to take a look at this. I'm not familiar with the Mailhandler module but I do have a practical use for it so I hope to pick it up shortly.
 

Log inorregister to post comments


 Dane Powell’s picture


 Dane Powell CreditAttribution: Dane Powell commented  2013 at 10:34pm 


Project:Mailhandler» Feeds
Component:Mailhandler» Code

There is precedent for closing security issues such as this one on the Processor side, rather than the Parser / caller side: https://drupal.org/node/1808832
 

Log inorregister to post comments


 Dane Powell’s picture


 Dane Powell CreditAttribution: Dane Powell commented  2013 at 10:35pm 


Also if this is truly a 'security issue' then it needs to go through the proper process for that, rather than being discussed publicly.
 

Log inorregister to post comments


 twistor’s picture


 twistor CreditAttribution: twistor commented 0:59pm 


Title:Allowed file extensions» Respect allowed file extensions in file mapper.
Assigned:Unassigned» twistor
Category:Feature request» Bug report
Priority:Normal» Major
Issue summary:View changes

Well, Feeds doesn't have a proper release, so the security process doesn't technically apply.

That said, this should be handled in Feeds file mapper.
 

Log inorregister to post comments


 twistor’s picture


 twistor CreditAttribution: twistor commented 1:05pm 


Related issues:+#706908: Enhance filefield mapper to perform more validation on remote URLs


Log inorregister to post comments


 twistor’s picture


 twistor CreditAttribution: twistor commented :01am 


Version:7.x-2.x-dev» 7.x-2.0-alpha8
Priority:Major» Critical
StatusFileSize
new10.29 KB
FAILED: [[SimpleTest]]: [MySQL] 4,447 pass(es), 5 fail(s), and 11 exception(s).
[ View ]


Log inorregister to post comments


 twistor’s picture


 twistor CreditAttribution: twistor commented :05am 


Status:Active» Needs review


Log inorregister to post comments



  


Status:Needs review» Needs work

The last submitted patch, 11: feeds-1848498-11.patch, failed testing.
 

Log inorregister to post comments


 twistor’s picture


 twistor CreditAttribution: twistor commented :25am 


Status:Needs work» Needs review
StatusFileSize
new11.93 KB
PASSED: [[SimpleTest]]: [MySQL] 4,455 pass(es).
[ View ]


Log inorregister to post comments


 twistor’s picture


 twistor CreditAttribution: twistor commented :45am 


Version:7.x-2.0-alpha8» 7.x-2.x-dev
StatusFileSize
new10.81 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in sites/default/modules/feeds/plugins/FeedsParser.inc.
[ View ]

Committed to 7.x-2.0-alpha9 branch.
 

Log inorregister to post comments



  


Status:Needs review» Needs work

The last submitted patch, 15: feeds-1848498-15.patch, failed testing.
 

Log inorregister to post comments


 twistor’s picture


 twistor CreditAttribution: twistor commented :50am 


Status:Needs work» Needs review
StatusFileSize
new501 bytes
new10.81 KB
PASSED: [[SimpleTest]]: [MySQL] 7,214 pass(es).
[ View ]


Log inorregister to post comments



  



twistor committed 9a197f1on7.x-2.x
Issue #1848498 by twistor: Respect allowed file extensions in file...


 

Log inorregister to post comments


 twistor’s picture


 twistor CreditAttribution: twistor commented :08am 


Status:Needs review» Fixed


Log inorregister to post comments


 Bevan’s picture


 Bevan CreditAttribution: Bevan commented m 


StatusFileSize
new11.36 KB
new11.36 KB

This patch is from the security.drupal.org issue node that led to SA-CONTRIB-2015-096, which covered multiple vulnerabilities in the Services module, one of which is the same as the vulnerability identified here.

This patch solves the problem using a solution crafted initially for Drupal 7 core. See #2472895: Provide file name sanitization functions. The new core functions have simply been copied and namespaced.

This patch is now redundant because @twistor has already solved this issue using a simpler patch. I am posting it solely for the purpose of making it public, now that there is no longer a reason to keep it private.
 

Log inorregister to post comments


 Bevan’s picture


 Bevan CreditAttribution: Bevan commented m 


Version:7.x-2.x-dev» 8.x-3.x-dev
Status:Fixed» Needs work

This issue likely applies to the Drupal 8 version of Feeds.

It might also apply to the Drupal 6 version if it is enhanced to respect the file field's configured path.

Feeds for Drupal 6 is not vulnerable, but only because it does not use the file field's path when building the destination directory. Since this is a feature that might be added, we should at least open an issue in the public issue queue after the SA is released.Security.drupal.org issue node

I am not sure if it is better to create new issue nodes for the two other versions that need investigation, or leave both on this issue node.
 

Log inorregister to post comments


 twistor’s picture


 twistor CreditAttribution: twistor commented :56am 


Version:8.x-3.x-dev» 6.x-1.x-dev

It doesn't exist in 8.x. http://cgit.drupalcode.org/feeds/tree/src/Feeds/Target/File.php?h=8.x-3....

As to 6.x, I highly doubt anyone is going to fix the bug that alleviates the security issue. But, we can leave this here to make sure.
 

Log inorregister to post comments

 










Related issues





#706908: Enhance filefield mapper to perform more validation on remote URLs 






Referenced by





#2511580: can't import commerce_file: The file [xxxx.xxx] has an invalid extension.
 











News Items

Drupal News

Planet Drupal

Association News

Social Media Directory

Security Announcements

Jobs

Our Community

Community

Getting Involved

Services, Training & Hosting

Groups & Meetups

DrupalCon

Code of Conduct

Online Documentation

Get Started

Documentation Home

Installation Guide

Site Building Guide

Support

api.drupal.org

Drupal Code Base

Download & Extend

Drupal Core

Modules

Themes

Distributions

Governance of Community

About

Web accessibility

The Drupal Association

About Drupal.org

Terms of Service

Privacy Policy




Drupal is a registered trademarkofDries Buytaert.