It appears this module doesn't handle:

* cross-site request forgeries to trick someone into deleting files
* user-quotas for the size of uploads and size of total uploads
* file types (e.g. that you shouldn't be able to upload php files)
* file_munge_filename to prevent more devious file extensions

It would be worth doing a thorough security review of the module prior to the 1.0 release.

Comments

compujohnny’s picture

compujohnny commented
Priority: Critical » Normal

Thanks for reporting these issues, we fixed the 1st, 3rd and 4th as these are the critical ones and we will work on the 2nd

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

Thanks for the quick work.

The change at http://drupalcode.org/project/jquery_file_upload.git/commitdiff/277b598?... seems insufficient since it is javascript it is client side and can be modified by the user. I suggest something server side.

The token in http://drupalcode.org/project/jquery_file_upload.git/commitdiff/da9ee62?... also feels insufficient - there should be token protection on all operations and I think the pairing of drupal_valid_token and drupal_get_token, with a seed based on the filename and operation, would be more robust.

compujohnny’s picture

compujohnny commented

Concerning the javascript change, since it's backed up by the file_munge_filename on the server side I think it will be OK,

I agree with you that the drupal_get_token approach is much better, I modified the code to use drupal_get_token and drupal_valid_token instead

Thanks for all your help :)

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

The file_munge_filename solution seems fine, though it doesn't give feedback to the user about why the file was rejected. That feels like more of a UX issue, though.

Thanks again for the quick action!

compujohnny’s picture

compujohnny commented

The regular user will get an error from the javascript if he tried to upload an unsupported extension

compujohnny’s picture

compujohnny commented
Status: Active » Fixed

Added configuration screen where file types and file sizes allowed for each user role can be configured, It supports max file size for each role, and total file quota for each role

Status: Fixed » Closed (fixed)

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