Skip to content

Commit

Permalink
[11.x] handle password_hash() failures better (#53821)
Browse files Browse the repository at this point in the history
* handle `password_hash()` failures better

as of PHP 8.0.0, `password_hash()` no longer returns false on failure, instead a `ValueError` will be thrown if the password hashing algorithm is not valid, or an `Error` if the password hashing failed for an unknown error.

I noticed the `ArgonHasher` already had a slightly updated check using error suppression, which was added in #33856.  however, I couldn't seem to get that to actually work, and for consistency I updated them both to use try/catch.

added tests for all 3 implementations to see if they correctly throw a `RuntimeException` if they are not able to be created.  I used invalid "cost" and "time" options to trigger this. not sure if there's a better way.

https://www.php.net/manual/en/function.password-hash.php#:~:text=the%20salt%20generation.-,8.0.0,-password_hash()%20no

* minor formatting
  • Loading branch information
browner12 authored Dec 10, 2024
1 parent ce0e481 commit 977e418
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 12 deletions.
15 changes: 8 additions & 7 deletions src/Illuminate/Hashing/ArgonHasher.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Illuminate\Hashing;

use Error;
use Illuminate\Contracts\Hashing\Hasher as HasherContract;
use RuntimeException;

Expand Down Expand Up @@ -60,13 +61,13 @@ public function __construct(array $options = [])
*/
public function make(#[\SensitiveParameter] $value, array $options = [])
{
$hash = @password_hash($value, $this->algorithm(), [
'memory_cost' => $this->memory($options),
'time_cost' => $this->time($options),
'threads' => $this->threads($options),
]);

if (! is_string($hash)) {
try {
$hash = password_hash($value, $this->algorithm(), [
'memory_cost' => $this->memory($options),
'time_cost' => $this->time($options),
'threads' => $this->threads($options),
]);
} catch (Error) {
throw new RuntimeException('Argon2 hashing not supported.');
}

Expand Down
11 changes: 6 additions & 5 deletions src/Illuminate/Hashing/BcryptHasher.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Illuminate\Hashing;

use Error;
use Illuminate\Contracts\Hashing\Hasher as HasherContract;
use RuntimeException;

Expand Down Expand Up @@ -44,11 +45,11 @@ public function __construct(array $options = [])
*/
public function make(#[\SensitiveParameter] $value, array $options = [])
{
$hash = password_hash($value, PASSWORD_BCRYPT, [
'cost' => $this->cost($options),
]);

if ($hash === false) {
try {
$hash = password_hash($value, PASSWORD_BCRYPT, [
'cost' => $this->cost($options),
]);
} catch (Error) {
throw new RuntimeException('Bcrypt hashing not supported.');
}

Expand Down
21 changes: 21 additions & 0 deletions tests/Hashing/HasherTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -117,4 +117,25 @@ public function testIsHashedWithNonHashedValue()
{
$this->assertFalse($this->hashManager->isHashed('foo'));
}

public function testBasicBcryptNotSupported()
{
$this->expectException(RuntimeException::class);

(new BcryptHasher(['rounds' => 0]))->make('password');
}

public function testBasicArgon2iNotSupported()
{
$this->expectException(RuntimeException::class);

(new ArgonHasher(['time' => 0]))->make('password');
}

public function testBasicArgon2idNotSupported()
{
$this->expectException(RuntimeException::class);

(new Argon2IdHasher(['time' => 0]))->make('password');
}
}

0 comments on commit 977e418

Please sign in to comment.