Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:69616
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=zqdbKzZNeLt-wTzd1jfdYSSZxw1Q4cqrN2Xok3BeAbpg@mail.gmail.com>
References:
	 <CA+GmM=yBFMwDS10HEeCEgDQJ7mZvDdiLXq7j3ABNC_641Lb94A@mail.gmail.com>
	 <1382013115.3980.236.camel@guybrush>
	 <CA+GmM=zqdbKzZNeLt-wTzd1jfdYSSZxw1Q4cqrN2Xok3BeAbpg@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Date: Thu, 17 Oct 2013 15:21:08 +0200
Message-ID: <1382016068.3980.264.camel@guybrush>
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
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 14:59 +0200, Jordi Boggiano wrote:
> On Thu, Oct 17, 2013 at 2:31 PM, Johannes Schlüter
> <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/ff8055fc5c9750482aac7a25a074aae0b1e64706 and some further commits, i.e.
> > https://github.com/php/php-src/commit/7126de4912d9d4c7499deb1f9239980400aa7ec7#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 = 'Foo\Bar\Type'.$userGivenType;
>   $obj = 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.

I know new $class() and other ways, I was more curious about the real
security impact -- usage of unchecked class names has quite a few
security implications even without autoloader. Are there "common" ways
about how unsafe class names might be injected?

An attacker could use that to trigger "bad" constructor behavior or
such.

> 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?

If PHP does a "minimal" check only there's nothing won. Autoloaders
still have to check according to their needs. For example on Windows
checking : is important, else I inject $classname = "c:\\foobar" as
filename and bypass one or the other autoloader. And who tells that
classes are loaded from filesystems, not database+eval, suddenly ' is
dangerous. There are tons of other examples easy to construct and we
should not pretend to be safe there.

If one takes class names from untrusted sources one has to be careful.

johannes