Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:69610
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain schlueters.de from 217.114.215.10 cause and error)
To: Jordi Boggiano <j.boggiano@seld.be>
Cc: PHP Developers Mailing List <internals@lists.php.net>
In-Reply-To: <CA+GmM=yBFMwDS10HEeCEgDQJ7mZvDdiLXq7j3ABNC_641Lb94A@mail.gmail.com>
References:
	 <CA+GmM=yBFMwDS10HEeCEgDQJ7mZvDdiLXq7j3ABNC_641Lb94A@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Date: Thu, 17 Oct 2013 14:31:55 +0200
Message-ID: <1382013115.3980.236.camel@guybrush>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Validation of class names in the autoload process
From: johannes@schlueters.de (Johannes =?ISO-8859-1?Q?Schl=FCter?=)

On Thu, 2013-10-17 at 09:27 +0200, Jordi Boggiano wrote:
> It has come to my attention that it's possible to abuse some
> autoloader by trying to autoload class names like
> "Foo\Bar\..\..\uploads\exploit.php" for example, which could be
> transformed to src/Foo/Bar/../../uploads/exploits.php, and then would
> require that file.

Are there codepaths where this can be injected from the outside? In the
unserializer this was prevented in
https://github.com/php/php-src/commit/ff8055fc5c9750482aac7a25a074aae0b1e64706 and some further commits, i.e.
https://github.com/php/php-src/commit/7126de4912d9d4c7499deb1f9239980400aa7ec7#diff-d697fc054b607bb0ffd7493daeb6a1afR616

Reasons against moving those in a more central place were

- Performance. checking always costs time (any autoloader is 
  way slower, shouldn't matter too much)
- There are more esoteric ways (mostly for extensions) to create
  classes with "illegal" names (only illegal class names I know are
  OCI-Lob and OCI-Collection from oci8 extension) an autoloader
  (inside an extension) might produce them (I think that is unlikely
  to exist in reality)
- Autoloader magic (developers sometimes do things I can't imagine)
- Autoloaders still have to verify (i.e. maximum length for the backend
  used, some backends might require additional protection (i.e. \ needs
  to be escaped))

I didn't consider the reasons to be strong, and I still don't. But maybe
I missed something.

For whitelisting "valid" classnames are described by

   [a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff\\]*


in the parser.

johannes