Skip to content

Commit 145ffd9

Browse files
cmb69cmb69
committed
Fix #78516: password_hash(): Memory cost is not in allowed range
libsodium measures the memory cost in bytes, while password_hash() and friends expect kibibyte values. We have to properly map between these scales not only when calling libsodium functions, but also when checking for allowed values. We also refactor to rid the code duplication.
1 parent c3376bf commit 145ffd9

File tree

3 files changed

+60
-48
lines changed

3 files changed

+60
-48
lines changed

‎NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ PHP NEWS
3232
- sodium:
3333
. Fixed bug #78510 (Partially uninitialized buffer returned by
3434
sodium_crypto_generichash_init()). (Frank Denis, cmb)
35+
. Fixed bug #78516 (password_hash(): Memory cost is not in allowed range).
36+
(cmb, Nikita)
3537

3638
- Standard:
3739
. Fixed bug #78506 (Error in a php_user_filter::filter() is not reported).

‎ext/sodium/sodium_pwhash.c

Lines changed: 40 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -40,40 +40,52 @@
4040
#define PHP_SODIUM_PWHASH_OPSLIMIT 4
4141
#define PHP_SODIUM_PWHASH_THREADS 1
4242

43+
static inline int get_options(zend_array *options, size_t *memlimit, size_t *opslimit) {
44+
zval *opt;
45+
46+
*opslimit = PHP_SODIUM_PWHASH_OPSLIMIT;
47+
*memlimit = PHP_SODIUM_PWHASH_MEMLIMIT << 10;
48+
if (!options) {
49+
return SUCCESS;
50+
}
51+
if ((opt = zend_hash_str_find(options, "memory_cost", strlen("memory_cost")))) {
52+
zend_long smemlimit = zval_get_long(opt);
53+
54+
if ((smemlimit < 0) || (smemlimit < crypto_pwhash_MEMLIMIT_MIN >> 10) || (smemlimit > (crypto_pwhash_MEMLIMIT_MAX >> 10))) {
55+
php_error_docref(NULL, E_WARNING, "Memory cost is outside of allowed memory range");
56+
return FAILURE;
57+
}
58+
*memlimit = smemlimit << 10;
59+
}
60+
if ((opt = zend_hash_str_find(options, "time_cost", strlen("time_cost")))) {
61+
*opslimit = zval_get_long(opt);
62+
if ((*opslimit < crypto_pwhash_OPSLIMIT_MIN) || (*opslimit > crypto_pwhash_OPSLIMIT_MAX)) {
63+
php_error_docref(NULL, E_WARNING, "Time cost is outside of allowed time range");
64+
return FAILURE;
65+
}
66+
}
67+
if ((opt = zend_hash_str_find(options, "threads", strlen("threads"))) && (zval_get_long(opt) != 1)) {
68+
php_error_docref(NULL, E_WARNING, "A thread value other than 1 is not supported by this implementation");
69+
return FAILURE;
70+
}
71+
return SUCCESS;
72+
}
73+
4374
static zend_string *php_sodium_argon2_hash(const zend_string *password, zend_array *options, int alg) {
44-
size_t opslimit = PHP_SODIUM_PWHASH_OPSLIMIT;
45-
size_t memlimit = PHP_SODIUM_PWHASH_MEMLIMIT;
75+
size_t opslimit, memlimit;
4676
zend_string *ret;
4777

4878
if ((ZSTR_LEN(password) >= 0xffffffff)) {
4979
php_error_docref(NULL, E_WARNING, "Password is too long");
5080
return NULL;
5181
}
5282

53-
if (options) {
54-
zval *opt;
55-
if ((opt = zend_hash_str_find(options, "memory_cost", strlen("memory_cost")))) {
56-
memlimit = zval_get_long(opt);
57-
if ((memlimit < crypto_pwhash_MEMLIMIT_MIN) || (memlimit > crypto_pwhash_MEMLIMIT_MAX)) {
58-
php_error_docref(NULL, E_WARNING, "Memory cost is outside of allowed memory range");
59-
return NULL;
60-
}
61-
}
62-
if ((opt = zend_hash_str_find(options, "time_cost", strlen("time_cost")))) {
63-
opslimit = zval_get_long(opt);
64-
if ((opslimit < crypto_pwhash_OPSLIMIT_MIN) || (opslimit > crypto_pwhash_OPSLIMIT_MAX)) {
65-
php_error_docref(NULL, E_WARNING, "Time cost is outside of allowed time range");
66-
return NULL;
67-
}
68-
}
69-
if ((opt = zend_hash_str_find(options, "threads", strlen("threads"))) && (zval_get_long(opt) != 1)) {
70-
php_error_docref(NULL, E_WARNING, "A thread value other than 1 is not supported by this implementation");
71-
return NULL;
72-
}
83+
if (get_options(options, &memlimit, &opslimit) == FAILURE) {
84+
return NULL;
7385
}
7486

7587
ret = zend_string_alloc(crypto_pwhash_STRBYTES - 1, 0);
76-
if (crypto_pwhash_str_alg(ZSTR_VAL(ret), ZSTR_VAL(password), ZSTR_LEN(password), opslimit, memlimit << 10, alg)) {
88+
if (crypto_pwhash_str_alg(ZSTR_VAL(ret), ZSTR_VAL(password), ZSTR_LEN(password), opslimit, memlimit, alg)) {
7789
php_error_docref(NULL, E_WARNING, "Unexpected failure hashing password");
7890
zend_string_release(ret);
7991
return NULL;
@@ -93,32 +105,12 @@ static zend_bool php_sodium_argon2_verify(const zend_string *password, const zen
93105
}
94106

95107
static zend_bool php_sodium_argon2_needs_rehash(const zend_string *hash, zend_array *options) {
96-
size_t opslimit = PHP_SODIUM_PWHASH_OPSLIMIT;
97-
size_t memlimit = PHP_SODIUM_PWHASH_MEMLIMIT;
98-
99-
if (options) {
100-
zval *opt;
101-
if ((opt = zend_hash_str_find(options, "memory_cost", strlen("memory_cost")))) {
102-
memlimit = zval_get_long(opt);
103-
if ((memlimit < crypto_pwhash_MEMLIMIT_MIN) || (memlimit > crypto_pwhash_MEMLIMIT_MAX)) {
104-
php_error_docref(NULL, E_WARNING, "Memory cost is outside of allowed memory range");
105-
return 1;
106-
}
107-
}
108-
if ((opt = zend_hash_str_find(options, "time_cost", strlen("time_cost")))) {
109-
opslimit = zval_get_long(opt);
110-
if ((opslimit < crypto_pwhash_OPSLIMIT_MIN) || (opslimit > crypto_pwhash_OPSLIMIT_MAX)) {
111-
php_error_docref(NULL, E_WARNING, "Time cost is outside of allowed time range");
112-
return 1;
113-
}
114-
}
115-
if ((opt = zend_hash_str_find(options, "threads", strlen("threads"))) && (zval_get_long(opt) != 1)) {
116-
php_error_docref(NULL, E_WARNING, "A thread value other than 1 is not supported by this implementation");
117-
return 1;
118-
}
119-
}
108+
size_t opslimit, memlimit;
120109

121-
return crypto_pwhash_str_needs_rehash(ZSTR_VAL(hash), opslimit, memlimit << 10);
110+
if (get_options(options, &memlimit, &opslimit) == FAILURE) {
111+
return 1;
112+
}
113+
return crypto_pwhash_str_needs_rehash(ZSTR_VAL(hash), opslimit, memlimit);
122114
}
123115

124116
static int php_sodium_argon2_get_info(zval *return_value, const zend_string *hash) {

‎ext/sodium/tests/bug78516.phpt

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
--TEST--
2+
Bug #78516 (password_hash(): Memory cost is not in allowed range)
3+
--SKIPIF--
4+
<?php
5+
if (!extension_loaded('sodium')) die('skip sodium extension not available');
6+
?>
7+
--FILE--
8+
<?php
9+
$pass = password_hash('secret', PASSWORD_ARGON2ID, ['memory_cost' => 8191]);
10+
password_needs_rehash($pass, PASSWORD_ARGON2ID, ['memory_cost' => 8191]);
11+
var_dump(password_get_info($pass)['options']['memory_cost']);
12+
$pass = password_hash('secret', PASSWORD_ARGON2I, ['memory_cost' => 8191]);
13+
password_needs_rehash($pass, PASSWORD_ARGON2I, ['memory_cost' => 8191]);
14+
var_dump(password_get_info($pass)['options']['memory_cost']);
15+
?>
16+
--EXPECT--
17+
int(8191)
18+
int(8191)

0 commit comments

Comments
 (0)