Skip to content

Commit c840f71

Browse files
TimWollasmalyshev
authored andcommitted
crypt: Fix validation of malformed BCrypt hashes
PHP’s implementation of crypt_blowfish differs from the upstream Openwall version by adding a “PHP Hack”, which allows one to cut short the BCrypt salt by including a `$` character within the characters that represent the salt. Hashes that are affected by the “PHP Hack” may erroneously validate any password as valid when used with `password_verify` and when comparing the return value of `crypt()` against the input. The PHP Hack exists since the first version of PHP’s own crypt_blowfish implementation that was added in 1e820ec. No clear reason is given for the PHP Hack’s existence. This commit removes it, because BCrypt hashes containing a `$` character in their salt are not valid BCrypt hashes.
1 parent 255e08a commit c840f71

File tree

2 files changed

+82
-8
lines changed

2 files changed

+82
-8
lines changed

ext/standard/crypt_blowfish.c

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,6 @@ static const unsigned char BF_atoi64[0x60] = {
371371
#define BF_safe_atoi64(dst, src) \
372372
{ \
373373
tmp = (unsigned char)(src); \
374-
if (tmp == '$') break; /* PHP hack */ \
375374
if ((unsigned int)(tmp -= 0x20) >= 0x60) return -1; \
376375
tmp = BF_atoi64[tmp]; \
377376
if (tmp > 63) return -1; \
@@ -399,13 +398,6 @@ static int BF_decode(BF_word *dst, const char *src, int size)
399398
*dptr++ = ((c3 & 0x03) << 6) | c4;
400399
} while (dptr < end);
401400

402-
if (end - dptr == size) {
403-
return -1;
404-
}
405-
406-
while (dptr < end) /* PHP hack */
407-
*dptr++ = 0;
408-
409401
return 0;
410402
}
411403

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
--TEST--
2+
bcrypt correctly rejects salts containing $
3+
--FILE--
4+
<?php
5+
for ($i = 0; $i < 23; $i++) {
6+
$salt = '$2y$04$' . str_repeat('0', $i) . '$';
7+
$result = crypt("foo", $salt);
8+
var_dump($salt);
9+
var_dump($result);
10+
var_dump($result === $salt);
11+
}
12+
?>
13+
--EXPECT--
14+
string(8) "$2y$04$$"
15+
string(2) "*0"
16+
bool(false)
17+
string(9) "$2y$04$0$"
18+
string(2) "*0"
19+
bool(false)
20+
string(10) "$2y$04$00$"
21+
string(2) "*0"
22+
bool(false)
23+
string(11) "$2y$04$000$"
24+
string(2) "*0"
25+
bool(false)
26+
string(12) "$2y$04$0000$"
27+
string(2) "*0"
28+
bool(false)
29+
string(13) "$2y$04$00000$"
30+
string(2) "*0"
31+
bool(false)
32+
string(14) "$2y$04$000000$"
33+
string(2) "*0"
34+
bool(false)
35+
string(15) "$2y$04$0000000$"
36+
string(2) "*0"
37+
bool(false)
38+
string(16) "$2y$04$00000000$"
39+
string(2) "*0"
40+
bool(false)
41+
string(17) "$2y$04$000000000$"
42+
string(2) "*0"
43+
bool(false)
44+
string(18) "$2y$04$0000000000$"
45+
string(2) "*0"
46+
bool(false)
47+
string(19) "$2y$04$00000000000$"
48+
string(2) "*0"
49+
bool(false)
50+
string(20) "$2y$04$000000000000$"
51+
string(2) "*0"
52+
bool(false)
53+
string(21) "$2y$04$0000000000000$"
54+
string(2) "*0"
55+
bool(false)
56+
string(22) "$2y$04$00000000000000$"
57+
string(2) "*0"
58+
bool(false)
59+
string(23) "$2y$04$000000000000000$"
60+
string(2) "*0"
61+
bool(false)
62+
string(24) "$2y$04$0000000000000000$"
63+
string(2) "*0"
64+
bool(false)
65+
string(25) "$2y$04$00000000000000000$"
66+
string(2) "*0"
67+
bool(false)
68+
string(26) "$2y$04$000000000000000000$"
69+
string(2) "*0"
70+
bool(false)
71+
string(27) "$2y$04$0000000000000000000$"
72+
string(2) "*0"
73+
bool(false)
74+
string(28) "$2y$04$00000000000000000000$"
75+
string(2) "*0"
76+
bool(false)
77+
string(29) "$2y$04$000000000000000000000$"
78+
string(2) "*0"
79+
bool(false)
80+
string(30) "$2y$04$0000000000000000000000$"
81+
string(60) "$2y$04$000000000000000000000u2a2UpVexIt9k3FMJeAVr3c04F5tcI8K"
82+
bool(false)

0 commit comments

Comments
 (0)