On 1 December 2015 at 09:50, Dmitry Stogov <dmi...@zend.com> wrote:
> Hi Nikita,
>
> few notes:
>
> It looks like the patch messes the attempt of catching the problem (few
> lines related to zend_hash_find_bucket changes) with optimization to
> compensate collision check overhead (everything else). I think it's better
> to separate these parts.
>
> Introduction of zend_hash_add_or_return() in 7.0.1 is going to make forward
> incompatibility with 7.0.0. However, we may reserve it for internal usage
> removing ZEND_API.
>
> I don't think PHP should prevent user from construction of arrays he likes,
> because of internal problems (like in your example).
>
> > $s = 1 << 16; $a = [];
> > for ($i = 0; count($a) < $s; $i += $s) { $a[$i] = 0; }
>
> It makes performance problem, but it doesn't mean we should kill it.
> We have "max_execution_time" to catch them all.
>
> I think only big arrays coming from external sources should be checked.
>
> Your solution is incomplete, anyway, because of crafting a single list with
> 10000 collisions, attacker will able to use N lists with 1000 collisions.
>
> Thanks. Dmitry.
This is a good point but bear in mind to get the same effect as for 10000
collisions, you'd need 100 lists not 10
