Feb 13, 2009

How I tightened PHP security and broke three extensions


Given the recent security threats with TYPO3 I decided to tighten security a little more on my servers. I already run Apache and other services in chrooted environment with very restricted permissions. Now I decided to implement open_basedir setting. I did not use it before because I know that many extensions are not written with open_basedir in mind. But I decided to risk.

It worked well first. I set open_basedir to "/path/to/site/root/:/path/to/typo3_src/". This is how this and other my sites run now. All was ok.

Today one of the visitors noticed that one particular page is blank. For any more or less experienced TYPO3 developer it means that a PHP error happened and was not shown to the user. I have set devIPmask to my IP address and displayErrors to 2. This way I always see all PHP errors while visitors do not see them.

I went to the page and was very surprised by see an error in an extension of mine, which was not updated for half a year or so. Definitely it broke due to the open_basedir. I turned open_basedir off and page went back to normal.

But what could break? I always program extensions so that they do not go anywhere outside of web root.A debug() call showed me that PHP glob() function suddenly started to return false. It should return an empty array if files were not found but with open_basedir it returned false! The directory in question was perfectly inside the web site root, it was not symlinked, etc.

I went to Google and found numerous complains about this behavior (this one was the best). It appears that one the PHP developers silently committed a change in one of PHP 5.2.x releases that broke many sites in the world. Just imagine:

if (is_dir($path) && is_readable($path)) {
  foreach (glob($path . '/*.whatever-non-existing') as $file) {
    // Do something
  }
}


This code perfectly works without open_basedir: glob() will return empty array. With open_basedir is_readable() will return false if directory is not accessible due to the open_basedir restriction. For my site it returns true but foreach fails because glob() now returns false instead of proper and documented empty array!

Of course numerous bugs were failed to PHP bug tracker. Of course PHP guys refused to admit they make a mistake. But they silently changed this behaviour back to normal in PHP 5.3 and they still do not admit the problem in PHP 5.2.x.

What could I do? I had to add checks for is_array()... Stupid to make such workarounds. But PHP developers think they are gods. Normal people have to make workarounds.

Result: do not use glob() or check its return value with is_array() even if you are globbing the existing and known path.

No comments:

Post a Comment