[narfbg]

Back to sscanf() usage, but with ironclad sanity checks and php_password_bcrypt_parse() can be used more explicitly as a validator, without attempting to extract cost for the caller.

Implements what I meant with #24203 (comment), somewhat with intent to also move this exception inside php_password_bcrypt_get_cost(), but it's a breaking change:

php-src/ext/standard/password.c

Lines 203 to 206 in 3c1a072

	if (cost < 4 || cost > 31) {
	zend_value_error("Invalid bcrypt cost parameter specified: " ZEND_LONG_FMT, cost);
	return NULL;
	}

[Sjord]

Please also test:

$2y$ 4$...
$2y$+4$...
$2y$\n4$...

[narfbg]

Good catch. If it's deemed more reliable (and maybe a touch more performant), we may switch to a for() loop-based scanner. sscanf() just makes the code a lot less cryptic IMO.

Edit: The failed checks seem to be unrelated flaky tests.

[TimWolla]

I'm extremely uninterested in (re)introducing the use of sscanf(). The general direction of the industry is getting rid of it (see curl as an example).

[narfbg]

Fair enough. Switched to a pure character validator.

[anun333]

Quoted below: written with Claude, an LLM, as CONTRIBUTING.md asks us to mark. The builds and test runs behind it are on my machine; the commands to repeat them are under the fold.

I tested this at the current head (bd93ce7066, with the default-cost change) on today's PHP-8.4 (7f8c655ecca). I compared password_get_info(), password_needs_rehash() and password_verify() between the two builds on the same hashes:

Input	PHP-8.4	this PR
685 real hashes from password_hash() and crypt() (details under the fold)	all 685 verify	identical results from all three functions
Every single-byte change of a valid cost-05 hash (60 positions × 255 values): password_get_info()	2y, cost 5, for 10,236 of them	unknown for those 10,236; the rest identical
The same 15,300: password_verify()	3 verify ($2a$, $2b$, $2x$ in place of $2y$)	the same 3
Cost written as 5, +5, \n5, 5 , -5, 03, 32, 99	unknown	unknown
password_needs_rehash($h, PASSWORD_BCRYPT), $h a malformed hash at cost 12 (4 tried)	false	true
This PR's password_get_info.phpt	FAIL (the two malformed segments)	PASS
This PR's password_needs_rehash.phpt	PASS	PASS
ext/standard/tests/password	12 passed, 0 failed	12 passed, 0 failed

The 10,236 newly reported unknown break down as:

• 10,176 have a byte outside ./0-9A-Za-z after the cost.
• 60 have a 22nd salt character other than ., O, e or u.

None of them verifies on either build, so a hash that works today is read the same way after this. crypt() writes only those four characters at position 28, whichever of the 64 its salt had there, so every hash PHP produced passes the new check.

One thing about the tests: the password_needs_rehash.phpt cases added for the malformed salt and hash segments pass on PHP-8.4 too. Their hashes are at cost 04, and PASSWORD_BCRYPT's default is 12, so both builds already say true. Asking for cost 4 makes them fail without the fix and pass with it:

var_dump(password_needs_rehash('$2y$04$MTIzNDU2Nzg5MDEyMzQ1NEj0NmcAWSLR.oP7XOR9HD/vjUuOj100y', PASSWORD_BCRYPT, ['cost' => 4]));
var_dump(password_needs_rehash('$2y$04$MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR+oP7XOR9HD/vjUuOj100y', PASSWORD_BCRYPT, ['cost' => 4]));

On PHP-8.4 both print bool(false), and with this PR both print bool(true). The same call on a well-formed cost-04 hash stays bool(false) on both.

How this was tested

Linux x86_64, gcc 13.3. Debug builds of origin/PHP-8.4 at 7f8c655ecca, and of the same commit with this PR's four commits cherry-picked:

./buildconf --force
./configure --enable-debug --disable-cgi --disable-phpdbg --disable-fileinfo --enable-pcntl --enable-zend-test --enable-fpm --with-curl --enable-mbstring --disable-mbregex --with-mysqli --with-pdo-mysql --with-openssl --with-bz2
make -j4

The real hashes, made once with the base build and read by both:

• password_hash() with PASSWORD_BCRYPT, 60 at each cost from 4 to 10 and 4 each at 11 and 12;
• crypt() with $2y$, $2a$, $2b$ and $2x$ salts, each with every one of the 64 characters as the last salt character;
• one SHA-512 crypt() hash.

These builds have no argon2, so there's no argon2 hash in the set.

For each hash, the script records:

• password_get_info()'s algo and cost;
• password_needs_rehash() with PASSWORD_BCRYPT, with no options and with ['cost' => '12'];
• password_verify() with the hash's password.

The mutations start from crypt('correct horse', '$2y$05$abcdefghijklmnopqrstuu') and replace one byte at a time with each of the other 255 values. Mutations of the cost digits that make the cost 13 or more skip password_verify() (a cost-25 hash takes hours on a debug build). The cost-12 rows used a non-canonical 22nd salt character, a + in the hash part, and this PR's two malformed test hashes with their cost changed to 12. The test folder ran with run-tests.php -n -q -j4 ext/standard/tests/password.