Dear developer team,

I noticed that nowhere in the code a validation of the uploaded file is done if it's really an image file or not. It's also possible to upload a file (.pdf) which was renamed to .jpg, for example ;-)

Here are the responsible lines which could be expanded as follows:

<?php
/**
 * Get the upload validators for an image field.
 *
 * @param $field CCK Field
 * @return array suitable for passing to file_save_upload() or the filefield
 *   element's '#upload_validators' property.
 */
function imagefield_widget_upload_validators($field) {
  $validators = array();
  if (!empty($field['widget']['max_resolution']) || !empty($field['widget']['min_resolution'])) {
    $validators['filefield_validate_image_resolution'] = array(
      $field['widget']['max_resolution'],
      $field['widget']['min_resolution'],
    );
  }
  $validators['file_validate_is_image'] = array(); // check if it's a real image file
  return $validators;
}
?>

Yours sincerely,

grandcat (developer of image fupload, soon with imagefield support =)

CommentFileSizeAuthor
#3 imagefield.module.patch550 bytesgrandcat
#3 filefield.module.patch543 bytesgrandcat

Comments

grandcat’s picture

grandcat commented
Status: Needs review » Active

Better replacement:

<?php
/**
* Get the upload validators for an image field.
*
* @param $field CCK Field
* @return array suitable for passing to file_save_upload() or the filefield
*   element's '#upload_validators' property.
*/
function imagefield_widget_upload_validators($field) {
  $validators = array();
  if (!empty($field['widget']['max_resolution']) || !empty($field['widget']['min_resolution'])) {
    $validators['filefield_validate_image_resolution'] = array(
      $field['widget']['max_resolution'],
      $field['widget']['min_resolution'],
    );
  }
  $validators['filefield_validate_is_image'] = array(); // check if it's a real image file
  return $validators;
}
?>

But it still throws the following error message:

Missing argument 1 for filefield_validate_is_image_help() in D:\...\htdocs\drupal6\sites\default\modules\filefield\filefield.module on line 665.

Argument "size" is missing, but I can't really recognize a sence of the need of this argument? See patch below which fixes this behaviour.

drewish’s picture

drewish commented

please post patches rather than code snippets. I think this is by design so that the field can accept rig and other formats that cannot be displayed by most browsers. That said with the new mime detection features we should be limiting it to image/* files.

grandcat’s picture

grandcat commented
StatusFileSize
new filefield.module.patch543 bytes
new imagefield.module.patch550 bytes

That said with the new mime detection features we should be limiting it to image/* files.

If a user doesn't want to install mimedetect, it falls back to function "file_get_mimetype", but only the file extension is checked. So, a renamed file could also be uploaded!
It's clear that a renamed PDF File can't be displayed as JPG, but this can become a security hole in future, so I advice you to add an additional validation function. This does not cost anything ;-)

I also added a patch for filefield which is needed to prevent an error message.

grandcat’s picture

grandcat commented
Status: Active » Needs review

Forgot to set status =/

drewish’s picture

drewish commented
Status: Active » Needs review

i'm not sure why the filefield.module patch is needed...

drewish’s picture

drewish commented
Status: Needs review » Fixed

ah, fixed the filefield portion of the issue with #300619: filefield_validate_is_image_help() should not have any parameters. committed the addition of the is_image validator.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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