Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:27433
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Message-ID: <22.23.20730.3B869A54@pb1.pair.com>
To: internals@lists.php.net
Date: Sat, 13 Jan 2007 23:15:51 +0000
User-Agent: Thunderbird 1.5.0.9 (Windows/20061207)
MIME-Version: 1.0
References: <F1076B95-2D05-4690-AE84-21FC0552F17A@ch2o.info> <45A8FC49.7050909@hardened-php.net>
In-Reply-To: <45A8FC49.7050909@hardened-php.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Comments on PHP security
From: tstarling@wikimedia.org (Tim Starling)

Stefan Esser wrote:
> It is very good that bugs in PHP Applications are bad for PHP's image.
> The majority of servers get hacked because of Remote URL Includes (and
> not by XSS/SQL Injection). The existance of Remote URL Includes is fully
> to blame on the PHP engine and is UNIQUE. After years the feature is
> still not kicked out of PHP. In PHP 5.2.1 there will be a configuration
> option that activates an (incomplete) blacklist. Everyone should know by
> now that blacklists are BAD.

Arbitrary local includes are almost as dangerous, because many PHP
applications provide an upload feature. Preventing remote includes is only
a small step towards security.

The reason PHP is particularly vulnerable is because subscripts are often
web-executable, and because of register_globals. When you put the two
together, you turn code that looks perfectly reasonable to an average
developer into a serious security vulnerability.

The code looks so reasonable, in fact, that most developers wouldn't
realise the problem unless they were told. And the manual doesn't tell
them the specific issue, not even in the security section.

I would like to see a prominent warning in the manual pages of include(),
require(), include_once() and require_once() stating that variables should
never be used as a base path for inclusion of subscripts, and that code
such as the following is extremely dangerous:

<?php
require_once("$InstallPath/BaseClass.php");
class DerivedClass extends BaseClass {
...

That way, at least those developers who read the manual will be protected.

-- Tim Starling