PHP Bugs Logo
PHP Bug

A few weeks ago we migrated a part of our hosting environment from PHP 5.3.6 to PHP 5.3.8. Normally an upgrade like this doesn’t cause any problems, since the PHP minor releases only contain bug- and security fixes. This time however, something big did change. The behaviour of the is_a function was radically altered, causing quite a few errors for clients using certain PHP/PEAR Frameworks. We quickly reverted it, investigated the issue and discovered not only the source of the change, but an alarming security bug that was introduced by it.

What was fixed?

PHP 5.3.7 included a fix for PHP bug #53727. This fix however changed the behavior of the is_a() function, a function normally used to check if a certain variable is a child of a specific Class. The original behavior accepted all sorts of inputs as its primary argument, including strings. The old behavior was to see if this “string” was an instance of a specific Class, which it obviously wasn’t, and return false. The new behavior however, attempts to be “helpful”, and passes its first argument to the __autoload() function. And it is this exact change that caused such unexpected behavior for our customers.

The problem our customers were having is that they had some (custom) code that implemented a very basic autoloader, in an attempt to reduce memory footprints by automatically loading class definitions when they were needed using the __autoload() function. Their code however never expected to be given anything other than a class name, but now all of a sudden they were receiving all sorts of objects. Take for example the following code snippet using a standard pear File library:

Normally one wouldn’t expect the __autoload() function to be called at all here, but the PEAR::File library uses the PEAR standards, and thus might return a PEAR::Error. As such, we need to use a call to PEAR::isError() to check if the file was read correctly or if an error was returned. PEAR::isError() checks this using the is_a function, and this ends up calling the autoloader function, which is obviously poorly equipped to handle anything but explicit classnames.
As a result, even this standard piece of PHP code, using standard libraries and code snippets from the php.net site itself suddenly has its behavior changed. Instead of simply sending the uploaded file to the process_upload() function, the __autoload() function now tries to include a file that doesn’t exist and throws a giant error to the client.

The problem with the new behavior

Normally a BC breaking bug isn’t a huge deal. Sure, some people have some unexpected behavior, which is why the developers try and avoid breaking BC in a minor update. If any does happen people file a bug and the behavior is fixed in the next update. The same happened to this bug: PHP bug #55475 was filed and a discussion was started about whether this bug should be fixed or if the change was intended behavior. The biggest problem with this new behavior however, is not just the fact that errors are suddenly displayed. Of course this was a problem for the webmasters hosting at our servers, but the real problem lies even deeper than that…

A lot of __autoload() implementations we found on our systems use the standard example from the php.net to include their classes, which doesn’t contain any sort of checks before trying to include a file. While one could argue that it’s never a good idea to simply copy example code into a live environment, this does happen more often than not, according to our scans. And it is exactly in this standard behavior that the problem lies.

If we look again at the example above, it’s easy to see what happens. A file, say a JPG file, is uploaded by the user and read from the disk. The script checks to see if it read the file correctly and in doing so passes the contents of the uploaded file to the __autoload() function, that tries to load the class. Now normally the server would print an error stating “Error: “include(/var/www/domain.com/upload.php) [function.include]: failed to open stream: No such file or directory” (error #2).”, and present this to the user.

The actual security bug

Now what if the user doesn’t upload an image file, but a carefully crafted text file with a JPEG extension. Imagine for example the following contents in the file:

Now we take that file and upload it to the website. The file is read by File::readAll(), its contents returned in the $uploaded_file. We pass this variable to the PEAR::isError() function, it passes it to the __autoload() function, which blindly prepares the string to include:

A nice and complete URL. It feeds this to the include() function which downloads the file with code from the remote website and, eventually, executes it.
At this point, you can consider your website lost, as the hacker can execute whatever code it wants on your website. He has full access to your database configuration file, your settings, your database with customer information, and everything else. The only recourse you have at this point is to restore your entire website from a known and trusted backup, change all your passwords (Both for your hosting environment, your website, and for all your customers who’s information has been exposed to the hackers.

The fix

Luckily there’s quite a few ways to fix it.

  • Disable the setting allow_url_include in your PHP.ini to prevent remote file inclusion
  • Patch your __autoload() to only include from a local dir;
  • Install Suhosin to protect yourself from remote file inclusion, and more.

Of course, the best fix for this is to not install PHP 5.3.7 or PHP 5.3.8 untill the PHP Project has fixed this bug and reverted to the old behavior

The impact

The impact of this bug is relatively small. It takes quite some specific code to get your input passed all the way to the is_a/autoloader, but as the example showed it’s possible to do. I personally don’t expect to see any exploits abusing this bug, but when it comes to the security of your website, better safe than sorry.

Also, for customers hosted at Byte, good news. Of course we have the allow_url_include setting turned off by default.

 

Update 2011-Sep-26: Replaced allow_url_fopen with allow_url_include, thanks Pierre!

Update 2011-Sep-27: Thanks to the guys at RedHat.com, CVE-2011-3379 has been assigned to this bug.

Update 2011-Sep-28: Added a link to the bug on php.net, added a note to clarify that customers at Byte aren’t threatened by this bug and a short impact analysis.

Update 2011-Nov-09: PHP has implemented a fix for the bug and implemented a BC compatible fix.

 

  • http://www.justanotherhacker.com Wireghoul

    Nice find!

  • http://blog.thepimp.net Pierre

    The correct setting is allow_url_include, not allow_url_fopen. Also it is important to note that such autoloader implementation is nothing else than include $_GET['somefile']; which is a well know very bad practice.

  • Cipriano Groenendal

    Hi guys, thanks for the comments!

    And yes, Pierre, in essence this is the same problem as the one you linked but with an important difference or two.

    Most good PHP developers know to protect such includes, but an autoloader is such a deep, internal, function that not many would think to add such protection to it. A simple scan on our hosting servers showed that nearly none of the autoloaders used did anything other than a simple “include $some_class”.
    And worse, the code for this autoloader can be copy/pasted straight off of php.net.

  • http://blog.thepimp.net Pierre

    Right, but it is the same issue.

    However the main part of my comment was about using the correct ini setting, being able to do file ops on remote streams is an important feature, so disabling only include with URL should be enough :)

  • Cipriano Groenendal

    You are absolutely right bout that, Pierre. I remember looking at both options and thought I had grabbed the right on, but I must’ve made a copy/paste error. I updated the blogpost to point to the right php.ini setting, thanks for the update!