[php-src] master: ext/standard: Validate the bcrypt cost before reading it

From: Date: Fri, 09 Oct 2026 11:19:35 +0000
Subject: [php-src] master: ext/standard: Validate the bcrypt cost before reading it
Groups: php.cvs 
Request: Send a blank email to php-cvs+get-142108@lists.php.net to get a copy of this message
Author: Ilia Alshanetsky (iliaal) Date: 2026-10-09T07:04:05-04:00 Commit: https://github.com/php/php-src/commit/2f7951d6490822ad91c13afc35b0c90e69a101a7 Raw diff: https://github.com/php/php-src/commit/2f7951d6490822ad91c13afc35b0c90e69a101a7.diff ext/standard: Validate the bcrypt cost before reading it password_get_info() and password_needs_rehash() identified any 60-byte "$2y" hash as bcrypt and read its cost with sscanf(), so they reported costs that crypt() rejects, such as 03 or 32, and overflowed zend_long on a run of 56 digits. Require exactly two digits in the 04-31 range followed by '$'. password_verify() is unaffected, as unidentified hashes still go through crypt(). Closes GH-24203 Changed paths: M NEWS M ext/standard/password.c M ext/standard/tests/password/password_get_info.phpt M ext/standard/tests/password/password_needs_rehash.phpt Diff: diff --git a/NEWS b/NEWS index f2c2a126aa6d..64d75dfc03ad 100644 --- a/NEWS +++ b/NEWS @@ -32,6 +32,8 @@ PHP NEWS - Standard: . Fixed chown() and lchown() failing to resolve user names in ZTS builds when getpwnam_r() needs a larger buffer. (Ilia Alshanetsky) + . Fixed password_get_info() and password_needs_rehash() accepting malformed + bcrypt costs. (Ilia Alshanetsky) - Zip: . Fixed use-after-free when re-entering ZipArchive during destruction or diff --git a/ext/standard/password.c b/ext/standard/password.c index 1e647bb301c3..9d6bd6f2003d 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -113,21 +113,33 @@ static zend_string* php_password_get_salt(zval *unused_, size_t required_salt_le /* bcrypt implementation */ -static bool php_password_bcrypt_valid(const zend_string *hash) { +static bool php_password_bcrypt_get_cost(const zend_string *hash, zend_long *cost) { const char *h = ZSTR_VAL(hash); - return (ZSTR_LEN(hash) == 60) && - (h[0] == '$') && (h[1] == '2') && (h[2] == 'y'); + + if ((ZSTR_LEN(hash) != 60) || + (h[0] != '$') || (h[1] != '2') || (h[2] != 'y') || (h[3] != '$') || + !ZEND_IS_DIGIT(h[4]) || !ZEND_IS_DIGIT(h[5]) || (h[6] != '$')) { + return false; + } + + *cost = (h[4] - '0') * 10 + (h[5] - '0'); + + return *cost >= 4 && *cost <= 31; +} + +static bool php_password_bcrypt_valid(const zend_string *hash) { + zend_long cost; + + return php_password_bcrypt_get_cost(hash, &cost); } static int php_password_bcrypt_get_info(zval *return_value, const zend_string *hash) { - zend_long cost = PHP_PASSWORD_BCRYPT_COST; + zend_long cost; - if (!php_password_bcrypt_valid(hash)) { + if (!php_password_bcrypt_get_cost(hash, &cost)) { /* Should never get called this way. */ return FAILURE; } - - sscanf(ZSTR_VAL(hash), "$2y$" ZEND_LONG_FMT "$", &cost); add_assoc_long(return_value, "cost", cost); return SUCCESS; @@ -135,15 +147,13 @@ static int php_password_bcrypt_get_info(zval *return_value, const zend_string *h static bool php_password_bcrypt_needs_rehash(const zend_string *hash, zend_array *options) { zval *znew_cost; - zend_long old_cost = PHP_PASSWORD_BCRYPT_COST; + zend_long old_cost; zend_long new_cost = PHP_PASSWORD_BCRYPT_COST; - if (!php_password_bcrypt_valid(hash)) { + if (!php_password_bcrypt_get_cost(hash, &old_cost)) { /* Should never get called this way. */ return 1; } - - sscanf(ZSTR_VAL(hash), "$2y$" ZEND_LONG_FMT "$", &old_cost); if (options && (znew_cost = zend_hash_str_find(options, "cost", sizeof("cost")-1)) != NULL) { new_cost = zval_get_long(znew_cost); } diff --git a/ext/standard/tests/password/password_get_info.phpt b/ext/standard/tests/password/password_get_info.phpt index 22c4ce4c52fa..29dd78fbc1f0 100644 --- a/ext/standard/tests/password/password_get_info.phpt +++ b/ext/standard/tests/password/password_get_info.phpt @@ -12,6 +12,26 @@ var_dump(password_get_info('$2y$11$MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR.oP7XOR9HD/vj // Test Non-Bcrypt var_dump(password_get_info('$1$rasmusle$rISCgZzpwk3UhDidwXvin0')); +// Valid cost boundaries +$suffix = 'MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR.oP7XOR9HD/vjUuOj100y'; +foreach ([4, 31] as $cost) { + $info = password_get_info('$2y$' . sprintf('%02d', $cost) . '$' . $suffix); + printf("valid %d: %s, cost %d\n", $cost, $info['algoName'], $info['options']['cost']); +} + +// Invalid cost grammar and range +$invalidHashes = [ + 'non-digit' => '$2y$a0$' . $suffix, + 'malformed separator' => '$2y$10x' . $suffix, + 'below range' => '$2y$03$' . $suffix, + 'above range' => '$2y$32$' . $suffix, + 'missing separator' => '$2y$' . str_repeat('9', 56), +]; +foreach ($invalidHashes as $description => $hash) { + $info = password_get_info($hash); + printf("%s: %s, options %d\n", $description, $info['algoName'], count($info['options'])); +} + echo "OK!"; ?> --EXPECT-- @@ -55,4 +75,11 @@ array(3) { array(0) { } } +valid 4: bcrypt, cost 4 +valid 31: bcrypt, cost 31 +non-digit: unknown, options 0 +malformed separator: unknown, options 0 +below range: unknown, options 0 +above range: unknown, options 0 +missing separator: unknown, options 0 OK! diff --git a/ext/standard/tests/password/password_needs_rehash.phpt b/ext/standard/tests/password/password_needs_rehash.phpt index d88270884e20..f8390996f234 100644 --- a/ext/standard/tests/password/password_needs_rehash.phpt +++ b/ext/standard/tests/password/password_needs_rehash.phpt @@ -33,6 +33,21 @@ var_dump(password_needs_rehash('$2y$'.$cost.'$MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR.o // Should Issue Needs Rehash, Since Foo is cast to 0... var_dump(password_needs_rehash('$2y$10$MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR.oP7XOR9HD/vjUuOj100y', PASSWORD_BCRYPT, array('cost' => 'foo'))); +// Valid cost boundaries +$suffix = 'MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR.oP7XOR9HD/vjUuOj100y'; +$costCases = [ + 'valid lower boundary' => ['$2y$04$' . $suffix, 4], + 'valid upper boundary' => ['$2y$31$' . $suffix, 31], + 'non-digit' => ['$2y$a0$' . $suffix, 12], + 'malformed separator' => ['$2y$10x' . $suffix, 10], + 'below range' => ['$2y$03$' . $suffix, 3], + 'above range' => ['$2y$32$' . $suffix, 32], +]; +foreach ($costCases as $description => $case) { + echo $description, ': '; + var_dump(password_needs_rehash($case[0], PASSWORD_BCRYPT, ['cost' => $case[1]])); +} + // CRYPT_MD5 var_dump(password_needs_rehash(crypt('Example', '$1$'), PASSWORD_DEFAULT)); @@ -54,6 +69,12 @@ bool(true) bool(true) bool(false) bool(true) +valid lower boundary: bool(false) +valid upper boundary: bool(false) +non-digit: bool(true) +malformed separator: bool(true) +below range: bool(true) +above range: bool(true) bool(true) bool(true) OK!

« previous php.cvs (#142108) next »