Skip to content

Commit 37c1171

Browse files
kocsismatekocsismate
committed
Promote warnings to exceptions in password_*() functions
1 parent d567688 commit 37c1171

File tree

7 files changed

+79
-54
lines changed

7 files changed

+79
-54
lines changed

‎ext/sodium/sodium_pwhash.c‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,20 +50,20 @@ static inline int get_options(zend_array *options, size_t *memlimit, size_t *ops
5050
zend_long smemlimit = zval_get_long(opt);
5151

5252
if ((smemlimit < 0) || (smemlimit < crypto_pwhash_MEMLIMIT_MIN >> 10) || (smemlimit > (crypto_pwhash_MEMLIMIT_MAX >> 10))) {
53-
php_error_docref(NULL, E_WARNING, "Memory cost is outside of allowed memory range");
53+
zend_value_error("Memory cost is outside of allowed memory range");
5454
return FAILURE;
5555
}
5656
*memlimit = smemlimit << 10;
5757
}
5858
if ((opt = zend_hash_str_find(options, "time_cost", strlen("time_cost")))) {
5959
*opslimit = zval_get_long(opt);
6060
if ((*opslimit < crypto_pwhash_OPSLIMIT_MIN) || (*opslimit > crypto_pwhash_OPSLIMIT_MAX)) {
61-
php_error_docref(NULL, E_WARNING, "Time cost is outside of allowed time range");
61+
zend_value_error("Time cost is outside of allowed time range");
6262
return FAILURE;
6363
}
6464
}
6565
if ((opt = zend_hash_str_find(options, "threads", strlen("threads"))) && (zval_get_long(opt) != 1)) {
66-
php_error_docref(NULL, E_WARNING, "A thread value other than 1 is not supported by this implementation");
66+
zend_value_error("A thread value other than 1 is not supported by this implementation");
6767
return FAILURE;
6868
}
6969
return SUCCESS;
@@ -74,7 +74,7 @@ static zend_string *php_sodium_argon2_hash(const zend_string *password, zend_arr
7474
zend_string *ret;
7575

7676
if ((ZSTR_LEN(password) >= 0xffffffff)) {
77-
php_error_docref(NULL, E_WARNING, "Password is too long");
77+
zend_value_error("Password is too long");
7878
return NULL;
7979
}
8080

@@ -84,7 +84,7 @@ static zend_string *php_sodium_argon2_hash(const zend_string *password, zend_arr
8484

8585
ret = zend_string_alloc(crypto_pwhash_STRBYTES - 1, 0);
8686
if (crypto_pwhash_str_alg(ZSTR_VAL(ret), ZSTR_VAL(password), ZSTR_LEN(password), opslimit, memlimit, alg)) {
87-
php_error_docref(NULL, E_WARNING, "Unexpected failure hashing password");
87+
zend_value_error("Unexpected failure hashing password");
8888
zend_string_release(ret);
8989
return NULL;
9090
}

‎ext/standard/basic_functions.stub.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1131,7 +1131,7 @@ function unpack(string $format, string $data, int $offset = 0): array|false {}
11311131

11321132
function password_get_info(string $hash): ?array {}
11331133

1134-
function password_hash(string $password, $algo, array $options = []): ?string {}
1134+
function password_hash(string $password, $algo, array $options = []): string {}
11351135

11361136
function password_needs_rehash(string $hash, $algo, array $options = []): bool {}
11371137

‎ext/standard/basic_functions_arginfo.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1741,7 +1741,7 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_password_get_info, 0, 1, IS_ARRA
17411741
ZEND_ARG_TYPE_INFO(0, hash, IS_STRING, 0)
17421742
ZEND_END_ARG_INFO()
17431743

1744-
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_password_hash, 0, 2, IS_STRING, 1)
1744+
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_password_hash, 0, 2, IS_STRING, 0)
17451745
ZEND_ARG_TYPE_INFO(0, password, IS_STRING, 0)
17461746
ZEND_ARG_INFO(0, algo)
17471747
ZEND_ARG_TYPE_INFO(0, options, IS_ARRAY, 0)

‎ext/standard/password.c‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -83,20 +83,20 @@ static zend_string* php_password_make_salt(size_t length) /* {{{ */
8383
zend_string *ret, *buffer;
8484

8585
if (length > (INT_MAX / 3)) {
86-
php_error_docref(NULL, E_WARNING, "Length is too large to safely generate");
86+
zend_value_error("Length is too large to safely generate");
8787
return NULL;
8888
}
8989

9090
buffer = zend_string_alloc(length * 3 / 4 + 1, 0);
9191
if (FAILURE == php_random_bytes_silent(ZSTR_VAL(buffer), ZSTR_LEN(buffer))) {
92-
php_error_docref(NULL, E_WARNING, "Unable to generate salt");
92+
zend_value_error("Unable to generate salt");
9393
zend_string_release_ex(buffer, 0);
9494
return NULL;
9595
}
9696

9797
ret = zend_string_alloc(length, 0);
9898
if (php_password_salt_to64(ZSTR_VAL(buffer), ZSTR_LEN(buffer), length, ZSTR_VAL(ret)) == FAILURE) {
99-
php_error_docref(NULL, E_WARNING, "Generated salt too short");
99+
zend_value_error("Generated salt too short");
100100
zend_string_release_ex(buffer, 0);
101101
zend_string_release_ex(ret, 0);
102102
return NULL;
@@ -193,7 +193,7 @@ static zend_string* php_password_bcrypt_hash(const zend_string *password, zend_a
193193
}
194194

195195
if (cost < 4 || cost > 31) {
196-
php_error_docref(NULL, E_WARNING, "Invalid bcrypt cost parameter specified: " ZEND_LONG_FMT, cost);
196+
zend_value_error("Invalid bcrypt cost parameter specified: " ZEND_LONG_FMT, cost);
197197
return NULL;
198198
}
199199

@@ -316,7 +316,7 @@ static zend_string *php_password_argon2_hash(const zend_string *password, zend_a
316316
}
317317

318318
if (memory_cost > ARGON2_MAX_MEMORY || memory_cost < ARGON2_MIN_MEMORY) {
319-
php_error_docref(NULL, E_WARNING, "Memory cost is outside of allowed memory range");
319+
zend_value_error("Memory cost is outside of allowed memory range");
320320
return NULL;
321321
}
322322

@@ -325,7 +325,7 @@ static zend_string *php_password_argon2_hash(const zend_string *password, zend_a
325325
}
326326

327327
if (time_cost > ARGON2_MAX_TIME || time_cost < ARGON2_MIN_TIME) {
328-
php_error_docref(NULL, E_WARNING, "Time cost is outside of allowed time range");
328+
zend_value_error("Time cost is outside of allowed time range");
329329
return NULL;
330330
}
331331

@@ -334,7 +334,7 @@ static zend_string *php_password_argon2_hash(const zend_string *password, zend_a
334334
}
335335

336336
if (threads > ARGON2_MAX_LANES || threads == 0) {
337-
php_error_docref(NULL, E_WARNING, "Invalid number of threads");
337+
zend_value_error("Invalid number of threads");
338338
return NULL;
339339
}
340340

@@ -374,7 +374,7 @@ static zend_string *php_password_argon2_hash(const zend_string *password, zend_a
374374

375375
if (status != ARGON2_OK) {
376376
zend_string_efree(encoded);
377-
php_error_docref(NULL, E_WARNING, "%s", argon2_error_message(status));
377+
zend_value_error("%s", argon2_error_message(status));
378378
return NULL;
379379
}
380380

@@ -650,7 +650,7 @@ PHP_FUNCTION(password_verify)
650650
}
651651
/* }}} */
652652

653-
/* {{{ proto string|null password_hash(string password, mixed algo[, array options = array()])
653+
/* {{{ proto string password_hash(string password, mixed algo[, array options = array()])
654654
Hash a password */
655655
PHP_FUNCTION(password_hash)
656656
{
@@ -669,15 +669,17 @@ PHP_FUNCTION(password_hash)
669669
algo = php_password_algo_find_zval(zalgo);
670670
if (!algo) {
671671
zend_string *algostr = zval_get_string(zalgo);
672-
php_error_docref(NULL, E_WARNING, "Unknown password hashing algorithm: %s", ZSTR_VAL(algostr));
672+
zend_value_error("Unknown password hashing algorithm: %s", ZSTR_VAL(algostr));
673673
zend_string_release(algostr);
674-
RETURN_NULL();
674+
return;
675675
}
676676

677677
digest = algo->hash(password, options);
678678
if (!digest) {
679-
/* algo->hash should have raised an error. */
680-
RETURN_NULL();
679+
if (!EG(exception)) {
680+
zend_throw_error(NULL, "Password hashing failed for unknown reason");
681+
}
682+
return;
681683
}
682684

683685
RETURN_NEW_STR(digest);

‎ext/standard/tests/password/password_bcrypt_errors.phpt‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,18 @@ Test error operation of password_hash() with bcrypt hashing
33
--FILE--
44
<?php
55
//-=-=-=-
6+
try {
7+
password_hash("foo", PASSWORD_BCRYPT, array("cost" => 3));
8+
} catch (ValueError $exception) {
9+
echo $exception->getMessage() . "\n";
10+
}
611

7-
var_dump(password_hash("foo", PASSWORD_BCRYPT, array("cost" => 3)));
8-
9-
var_dump(password_hash("foo", PASSWORD_BCRYPT, array("cost" => 32)));
10-
12+
try {
13+
var_dump(password_hash("foo", PASSWORD_BCRYPT, array("cost" => 32)));
14+
} catch (ValueError $exception) {
15+
echo $exception->getMessage() . "\n";
16+
}
1117
?>
12-
--EXPECTF--
13-
Warning: password_hash(): Invalid bcrypt cost parameter specified: 3 in %s on line %d
14-
NULL
15-
16-
Warning: password_hash(): Invalid bcrypt cost parameter specified: 32 in %s on line %d
17-
NULL
18+
--EXPECT--
19+
Invalid bcrypt cost parameter specified: 3
20+
Invalid bcrypt cost parameter specified: 32

‎ext/standard/tests/password/password_hash_error.phpt‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@ try {
1010
echo $e->getMessage(), "\n";
1111
}
1212

13-
var_dump(password_hash("foo", array()));
13+
try {
14+
password_hash("foo", array());
15+
} catch (ValueError $exception) {
16+
echo $exception->getMessage() . "\n";
17+
}
1418

1519
try {
1620
var_dump(password_hash("foo", 19, new StdClass));
@@ -35,9 +39,7 @@ try {
3539
password_hash() expects at least 2 parameters, 1 given
3640

3741
Warning: Array to string conversion in %s on line %d
38-
39-
Warning: password_hash(): Unknown password hashing algorithm: Array in %s on line %d
40-
NULL
42+
Unknown password hashing algorithm: Array
4143
password_hash() expects parameter 3 to be array, object given
4244
password_hash() expects parameter 3 to be array, string given
4345
password_hash() expects parameter 1 to be string, array given

‎ext/standard/tests/password/password_hash_error_argon2.phpt‎

Lines changed: 38 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,28 +7,46 @@ if (!defined('PASSWORD_ARGON2ID')) die('skip password_hash not built with Argon2
77
?>
88
--FILE--
99
<?php
10-
var_dump(password_hash('test', PASSWORD_ARGON2I, ['memory_cost' => 0]));
11-
var_dump(password_hash('test', PASSWORD_ARGON2I, ['time_cost' => 0]));
12-
var_dump(password_hash('test', PASSWORD_ARGON2I, ['threads' => 0]));
13-
var_dump(password_hash('test', PASSWORD_ARGON2ID, ['memory_cost' => 0]));
14-
var_dump(password_hash('test', PASSWORD_ARGON2ID, ['time_cost' => 0]));
15-
var_dump(password_hash('test', PASSWORD_ARGON2ID, ['threads' => 0]));
16-
?>
17-
--EXPECTF--
18-
Warning: password_hash(): Memory cost is outside of allowed memory range in %s on line %d
19-
NULL
10+
try {
11+
password_hash('test', PASSWORD_ARGON2I, ['memory_cost' => 0]);
12+
} catch (ValueError $exception) {
13+
echo $exception->getMessage() . "\n";
14+
}
2015

21-
Warning: password_hash(): Time cost is outside of allowed time range in %s on line %d
22-
NULL
16+
try {
17+
password_hash('test', PASSWORD_ARGON2I, ['time_cost' => 0]);
18+
} catch (ValueError $exception) {
19+
echo $exception->getMessage() . "\n";
20+
}
2321

24-
Warning: password_hash(): %sthread%s
25-
NULL
22+
try {
23+
password_hash('test', PASSWORD_ARGON2I, ['threads' => 0]);
24+
} catch (ValueError $exception) {
25+
echo $exception->getMessage() . "\n";
26+
}
2627

27-
Warning: password_hash(): Memory cost is outside of allowed memory range in %s on line %d
28-
NULL
28+
try {
29+
password_hash('test', PASSWORD_ARGON2ID, ['memory_cost' => 0]);
30+
} catch (ValueError $exception) {
31+
echo $exception->getMessage() . "\n";
32+
}
2933

30-
Warning: password_hash(): Time cost is outside of allowed time range in %s on line %d
31-
NULL
34+
try {
35+
password_hash('test', PASSWORD_ARGON2ID, ['time_cost' => 0]);
36+
} catch (ValueError $exception) {
37+
echo $exception->getMessage() . "\n";
38+
}
3239

33-
Warning: password_hash(): %sthread%s
34-
NULL
40+
try {
41+
password_hash('test', PASSWORD_ARGON2ID, ['threads' => 0]);
42+
} catch (ValueError $exception) {
43+
echo $exception->getMessage() . "\n";
44+
}
45+
?>
46+
--EXPECT--
47+
Memory cost is outside of allowed memory range
48+
Time cost is outside of allowed time range
49+
Invalid number of threads
50+
Memory cost is outside of allowed memory range
51+
Time cost is outside of allowed time range
52+
Invalid number of threads

0 commit comments

Comments
 (0)