Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:75054
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain sugarcrm.com designates 108.166.43.99 as permitted sender)
Message-ID: <53A92F93.2060507@sugarcrm.com>
Date: Tue, 24 Jun 2014 00:58:11 -0700
Organization: SugarCRM
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:24.0) Gecko/20100101 Thunderbird/24.6.0
MIME-Version: 1.0
To: Remi Collet <remi@fedoraproject.org>, 
 PHP Internals <internals@lists.php.net>
References: <CAH-PCH6xEXspisLOHtiTbViA4vDTk7q6LM3t_Bh9Dzr1cj_Nag@mail.gmail.com>	<53A1C722.9060501@fedoraproject.org>	<53A21137.6010705@sugarcrm.com>	<CAMUwpuT_S2EE6cdJM95RzY4vmORGx3vJ9+gau8G3EZCMsNSydA@mail.gmail.com>	<53A2A9BD.1070603@sugarcrm.com>	<CAH-PCH5-vtCVvGvceMqt6rDXq4zo0Vb34cyDK3KdBUNfg_oLyg@mail.gmail.com>	<CADyq6sKQbV+Avzfs3OAdGM9Hc85i18t0LSATD6Rk=HjqKOMgBg@mail.gmail.com>	<53A3874E.20704@sugarcrm.com>	<CAH-PCH6nSk0ir_TSjhCd0BVhiY5UsOsLKzqKhqfaU1G0ghi4Tg@mail.gmail.com>	<53A65578.6000701@sugarcrm.com> <CAH-PCH7tw0=OacG2HFDvBZxAho0HjdOtpLR=J9aXYGxsv40ehA@mail.gmail.com> <53A8626B.701@fedoraproject.org> <53A866B6.4060501@sugarcrm.com> <53A92B24.40706@fedoraproject.org>
In-Reply-To: <53A92B24.40706@fedoraproject.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Re: Bug 67072 resolution for 5.4/5.5
From: smalyshev@sugarcrm.com (Stas Malyshev)

Hi!

> I think the segfault have to be fixed in spl.

This can be done, however can we ensure all classes in PHP and
extensions would run properly when unserialized despite explicit
prohibition from the class to serialize/unserialize it?

Doing O: trick on such class is just not right. Note that crashes is
just the start of the problem - what if circumventing prohibited
unserialization puts the class into state that allows remote attacker to
trick it into doing something it's not supposed to do? Remember the
__dtor issue? That one worked on PHP level, this one would work on C
level, which would be much worse.

> And if we plan to allow newInstanceArgWithoutConstructor() for internal
> classes this is mandatory.

I'm not sure this is safe either, by the way. But at least here we don't
allow remote data to control our class' content and inject any data into
it without any controls whatsoever. So here might be a better way out of
this. But we need to be very careful with it.

> So we can allow "O:.." (perhaps only for empty data used in the
> phpunit/doctrine hack, => strlen(*p)<=1)

Right now we can not - it leads to remote-triggerable crash in any app
that unserializes outside data, and there are lots of these - just
search on github for unserialize($_POST or unserialize($_COOKIE. And I'm
not sure we can safely allow this in general. I'm sorry that this would
make a neat hack unavailable, but I think security of PHP apps is more
important than preserving this hack which was never documented and never
supposed to work in the first place.

I understand that this creates a need that we do not cover of how to
mock such objects, and I welcome suggestions - including how to make
newInstanceArgWithoutConstructor safe. But currently I do not see how we
can leave the unserialize hack in for classes like SplFileObject -
unless somebody points me to a way to make it safe.

-- 
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227