On 11/27/2015 9:12 AM, Nikita Popov wrote:
On Thu, Nov 26, 2015 at 8:35 PM, Niklas Keller <m...@kelunik.com> 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. Throwing an exception in this place would
be problematic, as we're inserting into hashtables all over the place and
not all of these places are exception-safe. Many places also make the
reasonable assumption that (normal) insert operations always succeed.
You are correct in saying that this approach would mean that you can now
easily trigger a fatal error in an application that was previously
susceptible to HashDos (and likely also in some that previously were not).
In most cases this should not be a problem, however for Aerys and likely
other daemons a fatal error implies losing all connections on the current
worker. This may effectively be worse than a HashDos attack.
The only thing I can think to offer here is to make the collision number
threshold ini configurable, so you would have the ability to effectively
disable this protection. In that case you'd have to manually perform size
checks in the right places if you wish to protect against HashDos.
I'm fine with having a solution that only works for the 99% of
applications, until such a time, if ever, as we can feasibly implement one
of the alternative schemes that would cover all possible scenarios.
Nikita
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 emergency hack and I've run
into the default 1,000 limit many times. When I hit that limit, I
inevitably have to raise it to anywhere from 3,000 to 10,000 to get the
target application to function, which, of course, puts the whole server
at risk.
If the application developer could declare what hash mode to use for new
arrays, that could potentially solve the problem more cleanly. SipHash
would be used for security scenarios (PHP loading of superglobals, user
input, etc.) and the current DJB hash for everything else. That could,
of course, lead to unfortunate situations. Alternatively, default to
SipHash for everything but the application developer could turn it off
and on as needed at runtime. Put a warning in the documentation about
the security of hashes and also mention leaving SipHash enabled when in
doubt.
--
Thomas Hruska
CubicleSoft President
I've got great, time saving software that you will find useful.
http://cubiclesoft.com/
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
