From: Christoph M. Becker Date: Sat, 14 Sep 2019 10:04:01 +0000 (+0200) Subject: Fix #78516: password_hash(): Memory cost is not in allowed range X-Git-Tag: php-7.4.0RC2~18 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=145ffd93fcac5fc04ae50464a34bc5e14fccc203;p=php 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. --- diff --git a/NEWS b/NEWS index dfec98cd4c..744c8f8754 100644 --- a/NEWS +++ b/NEWS @@ -32,6 +32,8 @@ PHP NEWS - sodium: . Fixed bug #78510 (Partially uninitialized buffer returned by sodium_crypto_generichash_init()). (Frank Denis, cmb) + . Fixed bug #78516 (password_hash(): Memory cost is not in allowed range). + (cmb, Nikita) - Standard: . Fixed bug #78506 (Error in a php_user_filter::filter() is not reported). diff --git a/ext/sodium/sodium_pwhash.c b/ext/sodium/sodium_pwhash.c index 7b7f574e17..2b284c7116 100644 --- a/ext/sodium/sodium_pwhash.c +++ b/ext/sodium/sodium_pwhash.c @@ -40,9 +40,39 @@ #define PHP_SODIUM_PWHASH_OPSLIMIT 4 #define PHP_SODIUM_PWHASH_THREADS 1 +static inline int get_options(zend_array *options, size_t *memlimit, size_t *opslimit) { + zval *opt; + + *opslimit = PHP_SODIUM_PWHASH_OPSLIMIT; + *memlimit = PHP_SODIUM_PWHASH_MEMLIMIT << 10; + if (!options) { + return SUCCESS; + } + if ((opt = zend_hash_str_find(options, "memory_cost", strlen("memory_cost")))) { + zend_long smemlimit = zval_get_long(opt); + + if ((smemlimit < 0) || (smemlimit < crypto_pwhash_MEMLIMIT_MIN >> 10) || (smemlimit > (crypto_pwhash_MEMLIMIT_MAX >> 10))) { + php_error_docref(NULL, E_WARNING, "Memory cost is outside of allowed memory range"); + return FAILURE; + } + *memlimit = smemlimit << 10; + } + if ((opt = zend_hash_str_find(options, "time_cost", strlen("time_cost")))) { + *opslimit = zval_get_long(opt); + if ((*opslimit < crypto_pwhash_OPSLIMIT_MIN) || (*opslimit > crypto_pwhash_OPSLIMIT_MAX)) { + php_error_docref(NULL, E_WARNING, "Time cost is outside of allowed time range"); + return FAILURE; + } + } + if ((opt = zend_hash_str_find(options, "threads", strlen("threads"))) && (zval_get_long(opt) != 1)) { + php_error_docref(NULL, E_WARNING, "A thread value other than 1 is not supported by this implementation"); + return FAILURE; + } + return SUCCESS; +} + static zend_string *php_sodium_argon2_hash(const zend_string *password, zend_array *options, int alg) { - size_t opslimit = PHP_SODIUM_PWHASH_OPSLIMIT; - size_t memlimit = PHP_SODIUM_PWHASH_MEMLIMIT; + size_t opslimit, memlimit; zend_string *ret; if ((ZSTR_LEN(password) >= 0xffffffff)) { @@ -50,30 +80,12 @@ static zend_string *php_sodium_argon2_hash(const zend_string *password, zend_arr return NULL; } - if (options) { - zval *opt; - if ((opt = zend_hash_str_find(options, "memory_cost", strlen("memory_cost")))) { - memlimit = zval_get_long(opt); - if ((memlimit < crypto_pwhash_MEMLIMIT_MIN) || (memlimit > crypto_pwhash_MEMLIMIT_MAX)) { - php_error_docref(NULL, E_WARNING, "Memory cost is outside of allowed memory range"); - return NULL; - } - } - if ((opt = zend_hash_str_find(options, "time_cost", strlen("time_cost")))) { - opslimit = zval_get_long(opt); - if ((opslimit < crypto_pwhash_OPSLIMIT_MIN) || (opslimit > crypto_pwhash_OPSLIMIT_MAX)) { - php_error_docref(NULL, E_WARNING, "Time cost is outside of allowed time range"); - return NULL; - } - } - if ((opt = zend_hash_str_find(options, "threads", strlen("threads"))) && (zval_get_long(opt) != 1)) { - php_error_docref(NULL, E_WARNING, "A thread value other than 1 is not supported by this implementation"); - return NULL; - } + if (get_options(options, &memlimit, &opslimit) == FAILURE) { + return NULL; } ret = zend_string_alloc(crypto_pwhash_STRBYTES - 1, 0); - if (crypto_pwhash_str_alg(ZSTR_VAL(ret), ZSTR_VAL(password), ZSTR_LEN(password), opslimit, memlimit << 10, alg)) { + if (crypto_pwhash_str_alg(ZSTR_VAL(ret), ZSTR_VAL(password), ZSTR_LEN(password), opslimit, memlimit, alg)) { php_error_docref(NULL, E_WARNING, "Unexpected failure hashing password"); zend_string_release(ret); return NULL; @@ -93,32 +105,12 @@ static zend_bool php_sodium_argon2_verify(const zend_string *password, const zen } static zend_bool php_sodium_argon2_needs_rehash(const zend_string *hash, zend_array *options) { - size_t opslimit = PHP_SODIUM_PWHASH_OPSLIMIT; - size_t memlimit = PHP_SODIUM_PWHASH_MEMLIMIT; - - if (options) { - zval *opt; - if ((opt = zend_hash_str_find(options, "memory_cost", strlen("memory_cost")))) { - memlimit = zval_get_long(opt); - if ((memlimit < crypto_pwhash_MEMLIMIT_MIN) || (memlimit > crypto_pwhash_MEMLIMIT_MAX)) { - php_error_docref(NULL, E_WARNING, "Memory cost is outside of allowed memory range"); - return 1; - } - } - if ((opt = zend_hash_str_find(options, "time_cost", strlen("time_cost")))) { - opslimit = zval_get_long(opt); - if ((opslimit < crypto_pwhash_OPSLIMIT_MIN) || (opslimit > crypto_pwhash_OPSLIMIT_MAX)) { - php_error_docref(NULL, E_WARNING, "Time cost is outside of allowed time range"); - return 1; - } - } - if ((opt = zend_hash_str_find(options, "threads", strlen("threads"))) && (zval_get_long(opt) != 1)) { - php_error_docref(NULL, E_WARNING, "A thread value other than 1 is not supported by this implementation"); - return 1; - } - } + size_t opslimit, memlimit; - return crypto_pwhash_str_needs_rehash(ZSTR_VAL(hash), opslimit, memlimit << 10); + if (get_options(options, &memlimit, &opslimit) == FAILURE) { + return 1; + } + return crypto_pwhash_str_needs_rehash(ZSTR_VAL(hash), opslimit, memlimit); } static int php_sodium_argon2_get_info(zval *return_value, const zend_string *hash) { diff --git a/ext/sodium/tests/bug78516.phpt b/ext/sodium/tests/bug78516.phpt new file mode 100644 index 0000000000..524b233518 --- /dev/null +++ b/ext/sodium/tests/bug78516.phpt @@ -0,0 +1,18 @@ +--TEST-- +Bug #78516 (password_hash(): Memory cost is not in allowed range) +--SKIPIF-- + +--FILE-- + 8191]); +password_needs_rehash($pass, PASSWORD_ARGON2ID, ['memory_cost' => 8191]); +var_dump(password_get_info($pass)['options']['memory_cost']); +$pass = password_hash('secret', PASSWORD_ARGON2I, ['memory_cost' => 8191]); +password_needs_rehash($pass, PASSWORD_ARGON2I, ['memory_cost' => 8191]); +var_dump(password_get_info($pass)['options']['memory_cost']); +?> +--EXPECT-- +int(8191) +int(8191)