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

Author: iliaal

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

ext/standard/password.c

@@ -113,37 +113,47 @@ 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;
 }
 
 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);
 	}

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!

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!