Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:114102
MIME-Version: 1.0
References: <CAESVnVpCK87GL=+nCvXuT7YS6-QPhF3=8LZSZuAusGFXoT006A@mail.gmail.com>
 <CAF+90c-imrQA4MRu9n6UMQio4jk_ferSaPk4O7wUOXZFAtcNcg@mail.gmail.com>
In-Reply-To: <CAF+90c-imrQA4MRu9n6UMQio4jk_ferSaPk4O7wUOXZFAtcNcg@mail.gmail.com>
Date: Thu, 22 Apr 2021 09:22:03 -0500
Message-ID: <CAESVnVqM8qAc+FMi_rRCzdrrvQdz6x=1RgV34xAGs+DdhhP1GQ@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: PHP internals <internals@lists.php.net>, Anthony Ferrara <ircmaxell@gmail.com>
Content-Type: multipart/alternative; boundary="000000000000e8130905c0906749"
Subject: Re: [PHP-DEV] Binary (un)safety of password_hash() used with PASSWORD_BCRYPT
From: pollita@php.net (Sara Golemon)

--000000000000e8130905c0906749
Content-Type: text/plain; charset="UTF-8"

On Thu, Apr 22, 2021 at 3:06 AM Nikita Popov <nikita.ppv@gmail.com> wrote:
> I don't think this is a good idea.
>
Fair, and that objection alone is enough to merit going RFC if I decide to
proceed with this.

> This adds an error condition that is based on the input string, which is
generally user-provided.
> As there is nothing in the default stack preventing null bytes from
passing in via HTTP (correct me if I'm wrong),
> this means the error can be trivially triggered, probably on about any
PHP application using password_hash().
>
Correct on all counts. However I see that as a good thing.  The site
developer no longer is left in a state where they have less security than
they think they have.  Say they have a minimum password length constraint
(quite common), and that check has passed, "Yes, the user came up with a
password that's at least 20 character long, good."  Now they pass that into
password_hash(), but because they didn't check for null bytes, the password
that actually gets hashed is only three bytes long, or maybe even an empty
string.  Nobody in the chain realizes this is the case, because why on
Earth would it be?

> I'm not sure what problem you're actually trying to solve here.
Ultimately, all this allows is a user to shoot themselves in the foot if
they want to -- and they do need to try actively, it's not like you can end
up with a null byte in a password by accident.
>

Niklas' reply just  prior to yours actually offers a case where that's not
unlikely.

> it's more likely the developer uses a hash function with raw output as a
pre-encoding,
> for reasons such as bringing long passwords below the bcrypt character
limit.

I hadn't actually considered this, but it makes null bytes showing up in an
input password not only possible, but reasonably likely.
In fact, there's a 1/256 chance that the lead byte would be null making
their effective password into an empty string from bcrypt's point of view.
This means that a brute force attack trying to login as that user would
also have a 1/256 chance of match it since the hashed version of a
completely dissimilar password is also empty.

The actual attack complexity has a worst case of 1/65536 since they have to
try lead-null hashable passwords on all users as they won't know which have
the unlucky property of having already hashed into a lead-null input.
Though if the pre-hash algorithm is known they could probably bring this
back to 1/256 by precomputing lead-null hashing inputs before scanning
users for a match.

Yes, this is something that can be addressed in userspace, but is it?  I
don't actually know the answer to that, but it worries me.

-Sara

--000000000000e8130905c0906749--