Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:88019
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.220.46 as permitted sender)
To: Yasuo Ohgaki <yohgaki@ohgaki.net>
References: <55E610C1.1000203@gmail.com>
 <CAGa2bXa41Jsh4vfNF+r3Mj2mLqPjT-UcgNgGzQ2U-d6MhwoMNw@mail.gmail.com>
 <CAGa2bXZtigz0dPe67HQJQnfjfnSNbnXP6m9Ut80BtXwF-8FUnA@mail.gmail.com>
Cc: PHP Internals <internals@lists.php.net>
Message-ID: <55E7D2E5.50104@gmail.com>
Date: Wed, 2 Sep 2015 21:56:05 -0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:38.0)
 Gecko/20100101 Thunderbird/38.2.0
MIME-Version: 1.0
In-Reply-To: <CAGa2bXZtigz0dPe67HQJQnfjfnSNbnXP6m9Ut80BtXwF-8FUnA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Heads up: merging security patches to 7
From: smalyshev@gmail.com (Stanislav Malyshev)

Hi!

> I see number of var_push_dtor() to fix unserialization.
> var_push_dtor() or var_push_dtor_no_addref() is required always when
> php_var_unserialize() is failed.
> Am I correct?

Not necessarily. Basically, what happens is that when you do
php_var_unserialize() the value you unserialize gets a slot in the
reference table. So if you are destroying it and some serialization part
later tries to access it - boom! That's where var_push_* comes in. The
first one adds refcount so even if everybody drops references to it the
value still survives. The second is used when we know no code uses this
value, so no need to add refcount - only thing that should keep it alive
is that it's still in the refs table (except for the case where it is
taken from the table by r: or R: - then its refcount is bumped).

Now the failure is tricky case - generally after the failure we need to
bail out ASAP, but that failure may not be the final failure -
serializes can be nested. So we may want to keep the values around even
if our unserialize fails, because encompassing serialize may still
reference that variable, since we may have already put it in the refs
table. We could make serialize failure a fatal error, but that'd be
pretty big BC break (exception wouldn't work, we'd need full blown fatal
E_ERROR).

In general, the idea is that anything that was put in reference table
(namely, anything that passed through php_var_unserialize()) should be
kept alive until the end of unserialize() call. Which means it should be
put to dtor list via var_push_dtor() or var_push_dtor_no_addref().

Now, that all was true for PHP 5. For PHP 7, the matter are more
complicated as the lists now keep zvals and not zval *, and I'm not 100%
sure yet what that means. I need to look into it and figure out.
-- 
Stas Malyshev
smalyshev@gmail.com