Skip to content

Commit 5d4d89e

Browse files
kocsismatekocsismate
committed
Promote warnings to exceptions in password_*() functions
1 parent 9563449 commit 5d4d89e

File tree

5 files changed

+69
-46
lines changed

5 files changed

+69
-46
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/password.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ 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

@@ -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

@@ -669,9 +669,9 @@ 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);

‎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)