Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:114090
MIME-Version: 1.0
Date: Wed, 21 Apr 2021 20:47:51 -0500
Message-ID: <CAESVnVpCK87GL=+nCvXuT7YS6-QPhF3=8LZSZuAusGFXoT006A@mail.gmail.com>
To: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000a097fc05c085decd"
Subject: Binary (un)safety of password_hash() used with PASSWORD_BCRYPT
From: pollita@php.net (Sara Golemon)

--000000000000a097fc05c085decd
Content-Type: text/plain; charset="UTF-8"

I have this notion that we've discussed this before, I'm certain I knew
that bcrypt wasn't binary safe, but someone reminded me that
password_hash() could be called with null bytes in the password itself and
that is just SCREAMING to have some safety-rails put on IMO.

So I've thrown together https://github.com/php/php-src/pull/6897 to test
that argon2 algos behave well (they do!), and modified the bcrypt algo to
throw an exception if you try to hash a new password containing a null, but
only warns if you've got a null when running password_verify().  My
reasoning for the latter is because anyone trying to auth with a null
character that succeeds does definitely know enough of the password to pass
by simply not passing the null, so we shouldn't break existing users who
already have hashes.  This only aims to break users who would otherwise be
able to include a null, because they would have a false sense of security
having their password truncated and can remedy that in their password
choosing.

Since this does introduce a (small) break, I'm putting it to the list for
opinions.
If nobody objects, I'll merge this (8.1 only) at the end of the month.

-Sara

--000000000000a097fc05c085decd--