Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:75037
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain sugarcrm.com designates 108.166.43.107 as permitted sender)
Message-ID: <53A65578.6000701@sugarcrm.com>
Date: Sat, 21 Jun 2014 21:03:04 -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: Ferenc Kovacs <tyra3l@gmail.com>, 
 Sebastian Bergmann <sebastian@php.net>
CC: Julien Pauli <jpauli@php.net>, 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>
In-Reply-To: <CAH-PCH6nSk0ir_TSjhCd0BVhiY5UsOsLKzqKhqfaU1G0ghi4Tg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Subject: Bug 67072 resolution for 5.4/5.5
From: smalyshev@sugarcrm.com (Stas Malyshev)

Hi!

We're getting pretty short on time for resolving 67072 one way or
another, since we want to release next week (we've accumulated quite a
lot of security issues). I still think that:

1. We can not leave unserialize() segfault in the code in 5.4
undisturbed. It's DoS opening as a minimum, and RCE potential, if
suitable internal class can be found, for every app that accepts
user-controlled serialized data. And there are a lot of such apps out there.

2. We do not want to break phpunit/doctrine, at least as much as
possible without hurting #1.

3. Current code does not fix the segfault problem entirely, e.g.:
class MySplFileObject extends SplFileObject {}
echo
unserialize('O:15:"MySplFileObject":1:{s:9:"*filename";s:15:"/home/flag/flag";}');

still segfaults for me.

4. There are other ways to cause segfaults with internal objects,
unfortunately, but those require deliberate coding and I'm less worried
about it since those have no external exploitation potential.

Given this, I am proposing to check the check in var_unserializer to this:

if (ce->serialize == NULL || ce->unserialize == zend_user_unserialize ||
(ZEND_INTERNAL_CLASS != ce->type && ce->create_object == NULL)) {
                object_init_ex(*rval, ce);
}

This would allow O: to work with the following:

1. Regular user classes
2. User classes implementing Serializable - the result will be
semantically broken but no segfault should be produced since it is still
all within PHP engine, so the worst is that some PHP variables will be
left null instead of their real values.
3. Internal classes that do not have their own serializer

The following will still not work with O::
1. Internal classes with their own serializer
2. User classes extending classes in 1.

I've checked the Horde/phpunit test by Remi, and it works fine, but I'd
like more feedback on this. Please note we need some solution one way or
another in 3 days, so if we need more discussion on this I'd advise
maybe set up some session on IRC to hash it out.

P.S. For 5.6, I'd just remove the usage of O: hack for any class that
produces C: in serialize. If serializer does not produce it, it should
not accept it.
-- 
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227