Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:69614
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain seld.be designates 209.85.223.178 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <1382013115.3980.236.camel@guybrush>
References: <CA+GmM=yBFMwDS10HEeCEgDQJ7mZvDdiLXq7j3ABNC_641Lb94A@mail.gmail.com>
	<1382013115.3980.236.camel@guybrush>
Date: Thu, 17 Oct 2013 14:59:02 +0200
Message-ID: <CA+GmM=zqdbKzZNeLt-wTzd1jfdYSSZxw1Q4cqrN2Xok3BeAbpg@mail.gmail.com>
To: =?ISO-8859-1?Q?Johannes_Schl=FCter?= <johannes@schlueters.de>
Cc: PHP Developers Mailing List <internals@lists.php.net>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Validation of class names in the autoload process
From: j.boggiano@seld.be (Jordi Boggiano)

On Thu, Oct 17, 2013 at 2:31 PM, Johannes Schl=FCter
<johannes@schlueters.de> wrote:
> 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/ff8055fc5c9750482aac7a25a074aae0b1e=
64706 and some further commits, i.e.
> https://github.com/php/php-src/commit/7126de4912d9d4c7499deb1f9239980400a=
a7ec7#diff-d697fc054b607bb0ffd7493daeb6a1afR616

Sorry when I said deserialize I meant this as a generic term not php's
unserialize(). I mean say you have two types of something, and the API
works taking a type param + some data for it, and you create it with
something along those lines:

  $class =3D 'Foo\Bar\Type'.$userGivenType;
  $obj =3D new $class($userData);

In this case, the autoloader would be called with whatever the user
passed in unless you validate it, but since autoloader injection isn't
a very common vector many devs wouldn't validate this and assume a
happy path I guess.

> 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))

IMO performance is the only valid concern, but it'll be faster done
once in the proper way in C than done in various crappy ways in every
userland autoloader, and it will also protect everyone at once. There
might be edge cases not covered by invalidating "." and "NUL", but to
keep it fast it may be best to only check for known bad chars instead
of the full regex?

Cheers