php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Doc Bug #77218 password_hash returns null on failure instead of false as of PHP 7.4
Submitted: 2018-11-29 08:26 UTC Modified: 2019-01-24 19:09 UTC
Votes:5
Avg. Score:3.8 ± 1.0
Reproduced:3 of 3 (100.0%)
Same Version:1 (33.3%)
Same OS:1 (33.3%)
From: magnar at myrtveit dot com Assigned:
Status: Open Package: *Encryption and hash functions
PHP Version: 7.3.0 OS:
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: magnar at myrtveit dot com
New email:
PHP Version: OS:

 

 [2018-11-29 08:26 UTC] magnar at myrtveit dot com
Description:
------------
From manual page: http://php.net/manual/en/function.password-hash.php

The return value is documented as "Returns the hashed password, or FALSE on failure." However, password_hash returns null on failure, as is evident from this test: https://3v4l.org/siaNi I am not sure whether password_hash returns false on other failures.

I don't know whether the issue is with the documentation or with the function.

Test script:
---------------
var_dump(password_hash('foo', -1));

Expected result:
----------------
false (based on the documentation)

Actual result:
--------------
null

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2018-11-29 08:33 UTC] magnar at myrtveit dot com
It seems that password_hash returns null on all failures. Here is my test: https://3v4l.org/DMv87
 [2018-12-01 13:15 UTC] petk@php.net
Hello, I'm just confirming this issue for now. Yes, the documentation should be probably fixed from false to null in case of failure such as non existing algorithm. Returning string or null is more logical in these more recently added functions. Returning mixed value of boolean is much less logical to expect and understand in such case I think.
 [2018-12-08 06:47 UTC] yohgaki@php.net
-Type: Documentation Problem +Type: Bug
 [2018-12-08 06:47 UTC] yohgaki@php.net
Briefly checked how RETURN_NULL() is used.
Most of them, but password_hash(), return NULL when "empty" result is appropriate, not for errors.

RETURN_NULL() for invalid algo seems actually a bug.
 [2019-01-02 13:55 UTC] nikic@php.net
-Type: Bug +Type: Documentation Problem
 [2019-01-02 13:55 UTC] nikic@php.net
password_hash() does indeed consistently use null for errors, so this should be adjusted in the docs, not implementation.
 [2019-01-02 14:32 UTC] cmb@php.net
@nikic This is true for master, but the code has recently been
changed[1], and apparently older versions would have returned
FALSE if the underlying hash function failed[2][3]][4]].  So
basically, for PHP ≤ 7.3, the function returned FALSE for a
failing implementation, and NULL for invalid parameters in
combination with a warning (the latter likely according to the
general convention to return NULL on ZPP failures).

This behavioral change looks rather dangerous to me, since
formerly developers who had carefully made sure that they pass
valid arguments might have checked for a FALSE return to signal
failure.  Now they'd get a NULL, which might pass their check.

[1] <https://github.com/php/php-src/commit/534df87c9e3c28001986e70844e0ad04e5708d3d>
[2] <https://github.com/php/php-src/blob/php-7.3.0/ext/standard/password.c#L492>
[3] <https://github.com/php/php-src/blob/php-7.3.0/ext/standard/password.c#L497>
[4] <https://github.com/php/php-src/blob/php-7.3.0/ext/standard/password.c#L585>
 [2019-01-24 18:10 UTC] girgias@php.net
-Summary: password_hash returns null +Summary: password_hash returns null on failure instead of false as of PHP 7.3 -Status: Open +Status: Feedback -Operating System: Any +Operating System: -PHP Version: 7.3.0RC6 +PHP Version: 7.3.0
 [2019-01-24 18:10 UTC] girgias@php.net
Is this going to be reverted or should I write a documentation patch?
 [2019-01-24 18:45 UTC] cmb@php.net
-Status: Feedback +Status: Open
 [2019-01-24 18:45 UTC] cmb@php.net
I think that might need discussion on internals@.

Anyhow, the status “feedback” is for requesting feedback from the
reporter – if no feedback is given after a week, the ticket will
automatically be closed with status “no feedback”.
 [2019-01-24 19:09 UTC] cmb@php.net
-Summary: password_hash returns null on failure instead of false as of PHP 7.3 +Summary: password_hash returns null on failure instead of false as of PHP 7.4
 [2019-01-24 19:09 UTC] cmb@php.net
Correction: the behavioral change does not affect PHP 7.3, but
master only.  The cases which return NULL in PHP 7.3 and before,
are according to the general note regarding return values for
invalid/unsuitable parameters[1].

In my opinion, password_hash() should *throw* on failure for the
same reasons random_bytes() does.

[1] <http://php.net/manual/en/functions.internal.php>
 [2019-02-17 04:08 UTC] weirdan at gmail dot com
> the behavioral change does not affect PHP 7.3, but master only. 

If there's a behavioral change, it should be mentioned in UPGRADING. 
7.4 branch seems to be missing such a note.
 [2019-12-02 21:27 UTC] weirdan at gmail dot com
> the behavioral change does not affect PHP 7.3, but master only. 

Ping. Master is 7.4 now and it's still needs to be documented.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Tue Mar 19 11:01:28 2024 UTC