Currently the destinatoin path of the preset is a fixed target - no way to change it using an API. Why this makes perfectly sense for public files, it really gives you hard times with private files.

Using the applied patch, which adds 1 line to the code you can fix the problem we have described here very easy: #796384: ImageCache makes private filesystem straightforward accesible (security issue #1172)
Imagecache implements its own api using this to actually place all "private" files (core implementation)

function imagecache_imagecache_preset_dest_alter(&$dest, $preset, $path) {  
  $matches = array();
  if(preg_match("@system/files/imagecache@", $path, $matches)) {
    // override completely
   // optionally secure directory automatically
   // _imagecache_add_htaccess(file_create_path(). "private");
    $dest = file_create_path(). "private/imagecache/$preset/$path";
  }
  // else this is no private file, dont change anything
}

This means that actually we devide public and private imagecache presets into to folders in files e.g. :
- imagecache/preset/path for the public
- private/imagecache/preset/path for the private

All you have to do now is to put a .htaccess with deny all file into private. That way there is no way to trick the security system. You can also do this automatically is its missing

function _imagecache_add_htaccess($path) {
  if(!file_check_directory($path)) {
    mkdir($path,0777,true);
  }
  $path = file_create_path( $path .'/.htaccess' );  
  file_save_data( "SetHandler This_is_a_Drupal_security_line_do_not_remove
Deny from all", $path, FILE_EXISTS_REPLACE );
  drupal_set_message("Added .htaccess file at $path to secure directory");
}

This approach (combined with http://drupal.org/node/809184 ) also works out for custom private file implementation which will run into the same issues.

This patch does not change the default behavior at all and does not include the code for fixing the imagecache private file (core) implementation.

CommentFileSizeAuthor
imagecache_dest_path.patch495 bytesEugenMayer

Comments

EugenMayer’s picture

EugenMayer commented

Actually one of the code snippets wont work. When we have a private file (core) we actually have the original local in $path when it gets passed to our hook. Actually in the core implementation we dont even need to check if the file is private, as its only globaly possible. 

function imagecache_imagecache_preset_dest_alter(&$dest, $preset, $path) {  
  switch (variable_get('file_downloads', FILE_DOWNLOADS_PUBLIC)) {
    case FILE_DOWNLOADS_PUBLIC:
      // do nothing
      return;
      break;
    case FILE_DOWNLOADS_PRIVATE:
      $dest = file_create_path(). "/private/imagecache/$preset/$path";
      return;
      break;  
  }
}

For other implementations like mine, where you can select file by file, if a file is private, you have to do a lookup here or check for a specific location or whatever you like. In any case this implementation works for complex implementatoins also.

In the end, it would be great if that hook would only be called if NOT the public-file handler has been called. Because for the public files this hook should never trigger at all, as this would make imagecache worthless ( as we dont have a mod_rewrite match with -f .. when we internally change the path ).

EugenMayer’s picture

EugenMayer commented
Status: Active » Reviewed & tested by the community
YK85’s picture

YK85 commented

subscribing

drewish’s picture

drewish commented
Status: Reviewed & tested by the community » Closed (won't fix)

#570132: Support CDN rewriting of ImageCache paths was committed and should allow you flexibility in this regard.