Image Handler 2 Security issue?

[Shane78]

McShane, the "777" is better interpreted as "writable". Back when IH2 was written, "777" was pretty much the only option that would work since PHP was typically configured to run in a way that required that in order to work. Nowadays "755" works for a growing number of server configurations.

The issue is not specific to IH2.

The bmz_cache folder needs to be "writable" by PHP because it's a PHP script that creates the files that get inserted into the bmz_cache folders and subfolders when it auto-sizes and/or watermarks the images it creates on-the-fly. If the folders aren't writable, it can't do its job, and thus the addon would be useless.

If *your* unique webserver setup allows lower permissions to still treat the files/folders as writable, then *you* can simply leave those files/folders at that level that works for you. Many hosts are now using a form of PHP running under CGI or suPHP which means "755" is sufficient to make the files/folders writable, without requiring them to be "777" they way other PHP configurations do.

The same principles apply to all PHP scripts, including addons and ZC core: a "writable" file is determined by the way the server and filesystem are configured.

So, yes, in the case of IH2 the specified folders need to be writable ... whatever that means for YOUR unique hosting configuration. Whether 777 or 755 or whatever.


Got it?

correct me if i am wrong.. i think you are saying
777 is bad... all mods that use 777 is bad... bad as in a naught fella can put naughty files in 777 folders...

so, as IH2 is creating 777 folders this can cause a weakness on a server that uses it??

[DrByte]

correct me if i am wrong.. i think you are saying
777 is bad... all mods that use 777 is bad... bad as in a naught fella can put naughty files in 777 folders...

so, as IH2 is creating 777 folders this can cause a weakness on a server that uses it??

No, I didn't say that.

Some mods, including some of the most popular ones, require "writable" folders. That's not necessarily bad. Even Zen Cart itself requires some writable files/folders for normal operation such as the admin uploading product images, and so on. There's nothing wrong with a mod requiring writable files/folders if there's good reason for it. Granted, where possible that should be avoided, but it's not a reason to call it naughty.

In the case of your fears, what's bad is using a hosting service that requires "777" in order to make the file/folder "writable by Apache/PHP" instead of being able to use a lower permission level to accomplish the same thing.

I'm not sure why you're bent on declaring all mods bad for some reason.

[Shane78]

i'll PM you

[DivaVocals]

Thanks for that DrByte.. In the case of IH2, it turns out that Tim's code was written to make sure that a wider variety of hosting configurations could use it. At that time there were many hosting services that required "777" in order to make the file/folder "writable by Apache/PHP". Though some of these hosts are still out there, things have changed a bit since Tim's codebase was released..

On some hosts, the folders INSIDE the bmz_cache folder will be created with 777 permissions. While on some hosts the folders inside the bmz_cache will inherit permissions from the parent (which should hopefully be set at 755).

I've been informed that this difference likely has something to do with how PHP is implemented some hosts server.

that is, whether it is running as an apache module

or

whether is run as CGI

After doing a little more digging, I discovered that this was the code which drives the directory creation:

/**
 * Creates a directory hierachy.
 *
 * @link    http://www.php.net/manual/en/function.mkdir.php
 * @author  <[email protected]>
 * @author  Andreas Gohr <[email protected]>
 * @author  Tim Kroeger <[email protected]>
 */
function io_mkdir_p($target){
  global $bmzConf;

  if (is_dir($target) || empty($target)) return 1; // best case check first
  if (@file_exists($target) && !is_dir($target)) return 0;
  //recursion
  if (io_mkdir_p(substr($target, 0, strrpos($target, '/')))){
    if($bmzConf['safemodehack']){
      $dir = preg_replace('/^' . preg_quote(realpath($bmzConf['ftp']['root']), '/') . '/', '', $target);
      return io_mkdir_ftp($dir);
    }else{
      return @mkdir($target, 0755); // crawl back up & create dir tree
    }
  }
  return 0;
}

Given that the majority or webhosts are now configured differently than when Tim wrote IH2 in 2006.. So it's possible that some of this code may be obsolete for current hosting configurations.

The changes that I suggested are as follows:

/**
 * Creates a directory hierachy.
 *
 * @link    http://www.php.net/manual/en/function.mkdir.php
 * @author  <[email protected]>
 * @author  Andreas Gohr <[email protected]>
 * @author  Tim Kroeger <[email protected]>
 */
function io_mkdir_p($target){
  global $bmzConf;

  if (is_dir($target) || empty($target)) return 1; // best case check first
  if (@file_exists($target) && !is_dir($target)) return 0;
  //recursion
  if (io_mkdir_p(substr($target, 0, strrpos($target, '/')))){
/*    if($bmzConf['safemodehack']){
      $dir = preg_replace('/^' . preg_quote(realpath($bmzConf['ftp']['root']), '/') . '/', '', $target);
      return io_mkdir_ftp($dir);
    }else{*/
      return @mkdir($target, 0755); // crawl back up & create dir tree
 //   }
  }
  return 0;
}

Still testing, but I think this ought to resolve the issue and allow IH2 to work across most hosting configurations.

Would love your insight sir..

No, I didn't say that.

Some mods, including some of the most popular ones, require "writable" folders. That's not necessarily bad. Even Zen Cart itself requires some writable files/folders for normal operation such as the admin uploading product images, and so on. There's nothing wrong with a mod requiring writable files/folders if there's good reason for it. Granted, where possible that should be avoided, but it's not a reason to call it naughty.

In the case of your fears, what's bad is using a hosting service that requires "777" in order to make the file/folder "writable by Apache/PHP" instead of being able to use a lower permission level to accomplish the same thing.

I'm not sure why you're bent on declaring all mods bad for some reason.

[DrByte]

It's worth a try.

Maybe McShane will volunteer to test it to prove that it's working satisfactorily?

[Shane78]

It's worth a try.

Maybe McShane will volunteer to test it to prove that it's working satisfactorily?

i would be more than happy to test. In fact, if you wish i can create you a webspace on one of my servers, give you FTP access and you can play yourself?

just let me know, it will take 10 mins to set up.
(but please note, its almost 7pm in newcastle, fast approching beer o clock... try an let me know before 7 otherwise if you want to take me up on the offer it wil be 2moro)

[DivaVocals]

It's worth a try.

Maybe McShane will volunteer to test it to prove that it's working satisfactorily?

Thanks.. I also asked ckosloff and a couple of other programmers to take a look at my proposed code changes.. I also have a few folks testing it as well..

The downside is that it's likely that older hosting configurations will not be able to use IH2 which is why we will likely just comment the code out (versus removing it altogether) This way those who need the original code can still use it (with a little instruction of course..)

[Shane78]

Thanks.. I also asked ckosloff and a couple of other programmers to take a look at my proposed code changes.. I also have a few folks testing it as well..

The downside is that it's likely that older hosting configurations will not be able to use IH2 which is why we will likely just comment the code out (versus removing it altogether) This way those who need the original code can still use it (with a little instruction of course..)

the offer is there if you want it

[DrByte]

Instead of commenting out the code, you could find out where the bmz safemode flag is set, and change the rules for that to be most compatible across hosting environments.