Re: [PHP-DEV] HashDos protection

2015-12-01 Thread Pierre Joye
On Dec 1, 2015 4:50 PM, "Dmitry Stogov" wrote: > > I think only big arrays coming from external sources should be checked. I tend to agree here. We discussed it with Remote last week. I was trying to explain why having a crafted hash function for inputs may be better and safer. That includes ge

Re: [PHP-DEV] HashDos protection

2015-12-01 Thread Dmitry Stogov
On Tue, Dec 1, 2015 at 3:48 PM, Nikita Popov wrote: > On Tue, Dec 1, 2015 at 10:50 AM, Dmitry Stogov 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 >> co

Re: [PHP-DEV] HashDos protection

2015-12-01 Thread Nikita Popov
On Tue, Dec 1, 2015 at 10:50 AM, Dmitry Stogov 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'

Re: [PHP-DEV] HashDos protection

2015-12-01 Thread Chris Riley
On 1 December 2015 at 09:50, Dmitry Stogov 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 be

Re: [PHP-DEV] HashDos protection

2015-12-01 Thread Dmitry Stogov
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_

Re: [PHP-DEV] HashDos protection

2015-12-01 Thread Remi Collet
Le 01/12/2015 02:28, Anatol Belski a écrit : >From what I was testing, the configuration is not absolutely necessary. My first thinking, seeing this hardcoded limit was also, "we need to make it an new ini option". So I run a few tests. Especially the Anatol one (a bit optimized to get more uni

RE: [PHP-DEV] HashDos protection

2015-11-30 Thread Anatol Belski
Hi Stas, > -Original Message- > From: Stanislav Malyshev [mailto:smalys...@gmail.com] > Sent: Tuesday, December 1, 2015 12:21 AM > To: Nikita Popov ; PHP internals > ; Anatol Belski ; Remi Collet > > Subject: Re: [PHP-DEV] HashDos protection > > Hi

Re: [PHP-DEV] HashDos protection

2015-11-30 Thread Stanislav Malyshev
Hi! > To fix the HashDos vulnerability for *all* cases (rather than just GET/POST > parsing), I propose to introduce collision counting during hashtable > insertion operations. This will throw a fatal error if the number of > collisions during an insertion operation exceed a certain threshold. >

Re: [PHP-DEV] HashDos protection

2015-11-29 Thread Levi Morrison
> 1. (Self-balancing binary trees). The idea here is that a balanced binary > tree has worst-case insertion time O(log N), while the linked list we > normally use has worst-case insertion time O(N). This means that the > worst-case execution time for N insertions into a hashtable goes from > O(N^2)

RE: [PHP-DEV] HashDos protection

2015-11-28 Thread Anatol Belski
Hi Thomas, > -Original Message- > From: Thomas Hruska [mailto:thru...@cubiclesoft.com] > Sent: Saturday, November 28, 2015 12:23 AM > To: Yasuo Ohgaki > Cc: PHP internals > Subject: Re: [PHP-DEV] HashDos protection > > On 11/27/2015 2:21 PM, Yasuo Ohg

Re: [PHP-DEV] HashDos protection

2015-11-28 Thread Nikita Popov
On Sat, Nov 28, 2015 at 12:22 AM, Thomas Hruska wrote: > On 11/27/2015 2:21 PM, Yasuo Ohgaki wrote: > >> Hi Thomas, >> >> In practice, we wouldn't have problems with max number of collisions. >> > > Is CLI going to be or can CLI be excluded from max collisions? After > thinking about it for a lo

RE: [PHP-DEV] HashDos protection

2015-11-27 Thread Anatol Belski
Hi Nikita, > -Original Message- > From: Nikita Popov [mailto:nikita@gmail.com] > Sent: Thursday, November 26, 2015 6:25 PM > To: PHP internals ; Anatol Belski > ; Remi Collet > Subject: [PHP-DEV] HashDos protection > > Hi internals! > > This mail tu

Re: [PHP-DEV] HashDos protection

2015-11-27 Thread Thomas Hruska
On 11/27/2015 2:21 PM, Yasuo Ohgaki wrote: Hi Thomas, In practice, we wouldn't have problems with max number of collisions. Is CLI going to be or can CLI be excluded from max collisions? After thinking about it for a long while, that's my only area of concern here. SAPI can (fatal) error t

Re: [PHP-DEV] HashDos protection

2015-11-27 Thread Yasuo Ohgaki
Hi Thomas, On Sat, Nov 28, 2015 at 2:07 AM, Thomas Hruska wrote: > I don't know if anyone has suggested this before, but why not have a > function that application developers can call to switch hash modes and > support multiple hash modes in the core? > > I've always viewed 'max_input_vars' as an

Re: [PHP-DEV] HashDos protection

2015-11-27 Thread Jakub Zelenka
Hi, On Thu, Nov 26, 2015 at 5:24 PM, Nikita Popov wrote: > > > What are your thoughts on this? > > First of all, thanks a lot for looking into it! That's great! I think that it's all cool except the fact that json_decode would result in fatal error. I don't think that json_decode should kill th

Re: [PHP-DEV] HashDos protection

2015-11-27 Thread Thomas Hruska
On 11/27/2015 9:12 AM, Nikita Popov wrote: On Thu, Nov 26, 2015 at 8:35 PM, Niklas Keller wrote: Would this be a catchable Error (implementing Throwable) or a real fatal? Having a real fatal could lead to a DOS in Aerys as you'd be able to crash workers with carefully crafted input variables t

Re: [PHP-DEV] HashDos protection

2015-11-27 Thread Nikita Popov
On Thu, Nov 26, 2015 at 8:35 PM, Niklas Keller wrote: > Would this be a catchable Error (implementing Throwable) or a real fatal? > Having a real fatal could lead to a DOS in Aerys as you'd be able to crash > workers with carefully crafted input variables then. > It would be a real fatal error.

Re: [PHP-DEV] HashDos protection

2015-11-27 Thread Niklas Keller
Currently it's not catchable, that's my main concern. If it's catchable, it's not that much of a problem. Regards, Niklas 2015-11-27 10:05 GMT+01:00 Yasuo Ohgaki : > Hi Nikita, > > On Fri, Nov 27, 2015 at 2:24 AM, Nikita Popov > wrote: > > What are your thoughts on this? > > Great! This is exac

Re: [PHP-DEV] HashDos protection

2015-11-27 Thread Yasuo Ohgaki
Hi Nikita, On Fri, Nov 27, 2015 at 2:24 AM, Nikita Popov wrote: > What are your thoughts on this? Great! This is exactly what I was thinking. I prefer collision counting rather than slower hash function. Hardcoded collision max (1000) seems ok for me. Catchable fatal error, at least E_RECOVERAB

Re: [PHP-DEV] HashDos protection

2015-11-27 Thread Leigh
On Thu, 26 Nov 2015 at 17:25 Nikita Popov wrote: > This will throw a fatal error if the number of > collisions during an insertion operation exceed a certain threshold. > To me this feels like it's just moving a DoS vector from one place to another. As Niklas already pointed out, he is directly

Re: [PHP-DEV] HashDos protection

2015-11-26 Thread Niklas Keller
> > 3. (Fatal error on many collisions). While the two previous options try to > ensure that hashtables stay efficient regardless of the used keys, the last > option aims to simply detect malicious array keys and abort the script in > such a case. > > This is done by counting the number of collisio

[PHP-DEV] HashDos protection

2015-11-26 Thread Nikita Popov
Hi internals! This mail turned out to be rather long, so I'll start with a TL;DR: To fix the HashDos vulnerability for *all* cases (rather than just GET/POST parsing), I propose to introduce collision counting during hashtable insertion operations. This will throw a fatal error if the number of c