Skip to content

Commit 6750f6c

Browse files
xabbuhxabbuh
committed
minor symfony#54858 [PasswordHasher] Make bcrypt nul byte hash test tolerant to PHP related failures (alexandre-daubois)
This PR was squashed before being merged into the 5.4 branch. Discussion ---------- [PasswordHasher] Make bcrypt nul byte hash test tolerant to PHP related failures | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT Bcrypt throws on PHP < 8.2 when passing a nul byte. The related test should be skipped for these versions as `$hasher->verify()` return value cannot be asserted. Related failure: https://github.com/symfony/symfony/actions/runs/8980661047/job/24664698662#step:7:1168 Commits ------- 30de18b [PasswordHasher] Make bcrypt nul byte hash test tolerant to PHP related failures
2 parents fbc47bc + 30de18b commit 6750f6c

File tree

2 files changed

+65
-9
lines changed

2 files changed

+65
-9
lines changed

‎src/Symfony/Component/PasswordHasher/Tests/Hasher/NativePasswordHasherTest.php

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,16 +98,44 @@ public function testBcryptWithLongPassword()
9898
$this->assertTrue($hasher->verify($hasher->hash($plainPassword), $plainPassword));
9999
}
100100

101-
public function testBcryptWithNulByte()
101+
/**
102+
* @requires PHP < 8.4
103+
*/
104+
public function testBcryptWithNulByteWithNativePasswordHash()
102105
{
103106
$hasher = new NativePasswordHasher(null, null, 4, \PASSWORD_BCRYPT);
104107
$plainPassword = "a\0b";
105108

106-
if (\PHP_VERSION_ID < 80218 || \PHP_VERSION_ID >= 80300 && \PHP_VERSION_ID < 80305) {
107-
// password_hash() does not accept passwords containing NUL bytes since PHP 8.2.18 and 8.3.5
108-
$this->assertFalse($hasher->verify(password_hash($plainPassword, \PASSWORD_BCRYPT, ['cost' => 4]), $plainPassword));
109+
try {
110+
$hash = password_hash($plainPassword, \PASSWORD_BCRYPT, ['cost' => 4]);
111+
} catch (\Throwable $throwable) {
112+
// we skip the test in case the current PHP version does not support NUL bytes in passwords
113+
// with bcrypt
114+
//
115+
// @see https://github.com/php/php-src/commit/11f2568767660ffe92fbc6799800e01203aad73a
116+
if (str_contains($throwable->getMessage(), 'Bcrypt password must not contain null character')) {
117+
$this->markTestSkipped('password_hash() does not accept passwords containing NUL bytes.');
118+
}
119+
120+
throw $throwable;
109121
}
110122

123+
if (null === $hash) {
124+
// we also skip the test in case password_hash() returns null as
125+
// implemented in security patches backports
126+
//
127+
// @see https://github.com/shivammathur/php-src-backports/commit/d22d9ebb29dce86edd622205dd1196a2796c08c7
128+
$this->markTestSkipped('password_hash() does not accept passwords containing NUL bytes.');
129+
}
130+
131+
$this->assertTrue($hasher->verify($hash, $plainPassword));
132+
}
133+
134+
public function testPasswordNulByteGracefullyHandled()
135+
{
136+
$hasher = new NativePasswordHasher(null, null, 4, \PASSWORD_BCRYPT);
137+
$plainPassword = "a\0b";
138+
111139
$this->assertTrue($hasher->verify($hasher->hash($plainPassword), $plainPassword));
112140
}
113141

‎src/Symfony/Component/PasswordHasher/Tests/Hasher/SodiumPasswordHasherTest.php

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -73,17 +73,45 @@ public function testBcryptWithLongPassword()
7373
$this->assertTrue($hasher->verify((new NativePasswordHasher(null, null, 4, \PASSWORD_BCRYPT))->hash($plainPassword), $plainPassword));
7474
}
7575

76-
public function testBcryptWithNulByte()
76+
/**
77+
* @requires PHP < 8.4
78+
*/
79+
public function testBcryptWithNulByteWithNativePasswordHash()
7780
{
7881
$hasher = new SodiumPasswordHasher(null, null);
7982
$plainPassword = "a\0b";
8083

81-
if (\PHP_VERSION_ID < 80218 || \PHP_VERSION_ID >= 80300 && \PHP_VERSION_ID < 80305) {
82-
// password_hash() does not accept passwords containing NUL bytes since PHP 8.2.18 and 8.3.5
83-
$this->assertFalse($hasher->verify(password_hash($plainPassword, \PASSWORD_BCRYPT, ['cost' => 4]), $plainPassword));
84+
try {
85+
$hash = password_hash($plainPassword, \PASSWORD_BCRYPT, ['cost' => 4]);
86+
} catch (\Throwable $throwable) {
87+
// we skip the test in case the current PHP version does not support NUL bytes in passwords
88+
// with bcrypt
89+
//
90+
// @see https://github.com/php/php-src/commit/11f2568767660ffe92fbc6799800e01203aad73a
91+
if (str_contains($throwable->getMessage(), 'Bcrypt password must not contain null character')) {
92+
$this->markTestSkipped('password_hash() does not accept passwords containing NUL bytes.');
93+
}
94+
95+
throw $throwable;
8496
}
8597

86-
$this->assertTrue($hasher->verify((new NativePasswordHasher(null, null, 4, \PASSWORD_BCRYPT))->hash($plainPassword), $plainPassword));
98+
if (null === $hash) {
99+
// we also skip the test in case password_hash() returns null as
100+
// implemented in security patches backports
101+
//
102+
// @see https://github.com/shivammathur/php-src-backports/commit/d22d9ebb29dce86edd622205dd1196a2796c08c7
103+
$this->markTestSkipped('password_hash() does not accept passwords containing NUL bytes.');
104+
}
105+
106+
$this->assertTrue($hasher->verify($hash, $plainPassword));
107+
}
108+
109+
public function testPasswordNulByteGracefullyHandled()
110+
{
111+
$hasher = new SodiumPasswordHasher(null, null);
112+
$plainPassword = "a\0b";
113+
114+
$this->assertTrue($hasher->verify($hasher->hash($plainPassword), $plainPassword));
87115
}
88116

89117
public function testUserProvidedSaltIsNotUsed()

0 commit comments

Comments
 (0)