From ad6c8991b37ee89c179f91f83e9255b9620b4276 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 10 Sep 2019 11:36:07 +0200 Subject: [PATCH 1/5] Fix #78516: password_hash(): Memory cost is outside of allowed memory 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. --- ext/sodium/sodium_pwhash.c | 22 +++++++++++++--------- ext/sodium/tests/bug78516.phpt | 18 ++++++++++++++++++ 2 files changed, 31 insertions(+), 9 deletions(-) create mode 100644 ext/sodium/tests/bug78516.phpt diff --git a/ext/sodium/sodium_pwhash.c b/ext/sodium/sodium_pwhash.c index 7b7f574e173a9..43360f17021ca 100644 --- a/ext/sodium/sodium_pwhash.c +++ b/ext/sodium/sodium_pwhash.c @@ -42,7 +42,7 @@ 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 memlimit = PHP_SODIUM_PWHASH_MEMLIMIT << 10; zend_string *ret; if ((ZSTR_LEN(password) >= 0xffffffff)) { @@ -53,11 +53,13 @@ static zend_string *php_sodium_argon2_hash(const zend_string *password, zend_arr 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)) { + 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 NULL; } + memlimit = smemlimit << 10; } if ((opt = zend_hash_str_find(options, "time_cost", strlen("time_cost")))) { opslimit = zval_get_long(opt); @@ -73,7 +75,7 @@ static zend_string *php_sodium_argon2_hash(const zend_string *password, zend_arr } 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; @@ -94,16 +96,18 @@ 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; + size_t memlimit = PHP_SODIUM_PWHASH_MEMLIMIT << 10; 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)) { + 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 1; } + memlimit = smemlimit << 10; } if ((opt = zend_hash_str_find(options, "time_cost", strlen("time_cost")))) { opslimit = zval_get_long(opt); @@ -118,13 +122,13 @@ static zend_bool php_sodium_argon2_needs_rehash(const zend_string *hash, zend_ar } } - return crypto_pwhash_str_needs_rehash(ZSTR_VAL(hash), opslimit, memlimit << 10); + 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) { const char* p = NULL; zend_long v = 0, threads = 1; - zend_long memory_cost = PHP_SODIUM_PWHASH_MEMLIMIT; + zend_long memory_cost = PHP_SODIUM_PWHASH_MEMLIMIT << 10; zend_long time_cost = PHP_SODIUM_PWHASH_OPSLIMIT; if (!hash || (ZSTR_LEN(hash) < sizeof("$argon2id$"))) { diff --git a/ext/sodium/tests/bug78516.phpt b/ext/sodium/tests/bug78516.phpt new file mode 100644 index 0000000000000..fd4b368d798fa --- /dev/null +++ b/ext/sodium/tests/bug78516.phpt @@ -0,0 +1,18 @@ +--TEST-- +Bug #78516 (password_hash(): Memory cost is outside of allowed memory 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) \ No newline at end of file From 90288c49aaac96bff7046a88690404919830d961 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 10 Sep 2019 13:32:27 +0200 Subject: [PATCH 2/5] Add trailing linefeed --- ext/sodium/tests/bug78516.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/sodium/tests/bug78516.phpt b/ext/sodium/tests/bug78516.phpt index fd4b368d798fa..a4f5b419727c2 100644 --- a/ext/sodium/tests/bug78516.phpt +++ b/ext/sodium/tests/bug78516.phpt @@ -15,4 +15,4 @@ var_dump(password_get_info($pass)['options']['memory_cost']); ?> --EXPECT-- int(8191) -int(8191) \ No newline at end of file +int(8191) From bfc1c4321fa58cd53620040d97b735412b3298ce Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Fri, 13 Sep 2019 17:59:10 +0200 Subject: [PATCH 3/5] Extract function --- ext/sodium/sodium_pwhash.c | 66 ++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 38 deletions(-) diff --git a/ext/sodium/sodium_pwhash.c b/ext/sodium/sodium_pwhash.c index 43360f17021ca..6f8e4caf390a8 100644 --- a/ext/sodium/sodium_pwhash.c +++ b/ext/sodium/sodium_pwhash.c @@ -40,6 +40,32 @@ #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; + + 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 << 10; @@ -51,25 +77,7 @@ static zend_string *php_sodium_argon2_hash(const zend_string *password, zend_arr } if (options) { - zval *opt; - 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 NULL; - } - 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 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"); + if (get_options(options, &memlimit, &opslimit) == FAILURE) { return NULL; } } @@ -99,25 +107,7 @@ static zend_bool php_sodium_argon2_needs_rehash(const zend_string *hash, zend_ar size_t memlimit = PHP_SODIUM_PWHASH_MEMLIMIT << 10; if (options) { - zval *opt; - 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 1; - } - 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 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"); + if (get_options(options, &memlimit, &opslimit) == FAILURE) { return 1; } } From b7a076c39964d389152f107a4a2b01f85a9ac167 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Sat, 14 Sep 2019 11:00:55 +0200 Subject: [PATCH 4/5] Move default option value handling to get_options() --- ext/sodium/sodium_pwhash.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/ext/sodium/sodium_pwhash.c b/ext/sodium/sodium_pwhash.c index 6f8e4caf390a8..097655a98b1e6 100644 --- a/ext/sodium/sodium_pwhash.c +++ b/ext/sodium/sodium_pwhash.c @@ -43,6 +43,11 @@ 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); @@ -67,8 +72,7 @@ static inline int get_options(zend_array *options, size_t *memlimit, size_t *ops } 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 << 10; + size_t opslimit, memlimit; zend_string *ret; if ((ZSTR_LEN(password) >= 0xffffffff)) { @@ -76,10 +80,8 @@ static zend_string *php_sodium_argon2_hash(const zend_string *password, zend_arr return NULL; } - if (options) { - if (get_options(options, &memlimit, &opslimit) == FAILURE) { - return NULL; - } + if (get_options(options, &memlimit, &opslimit) == FAILURE) { + return NULL; } ret = zend_string_alloc(crypto_pwhash_STRBYTES - 1, 0); @@ -103,15 +105,11 @@ 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 << 10; + size_t opslimit, memlimit; - if (options) { - if (get_options(options, &memlimit, &opslimit) == FAILURE) { - return 1; - } + if (get_options(options, &memlimit, &opslimit) == FAILURE) { + return 1; } - return crypto_pwhash_str_needs_rehash(ZSTR_VAL(hash), opslimit, memlimit); } From 292cb6b5fdb97236ac5b64fc18bf2193954e6e7b Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Sat, 14 Sep 2019 12:04:01 +0200 Subject: [PATCH 5/5] Don't multiply default by 1k when getting hash info --- ext/sodium/sodium_pwhash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/sodium/sodium_pwhash.c b/ext/sodium/sodium_pwhash.c index 097655a98b1e6..2b284c711685a 100644 --- a/ext/sodium/sodium_pwhash.c +++ b/ext/sodium/sodium_pwhash.c @@ -116,7 +116,7 @@ static zend_bool php_sodium_argon2_needs_rehash(const zend_string *hash, zend_ar static int php_sodium_argon2_get_info(zval *return_value, const zend_string *hash) { const char* p = NULL; zend_long v = 0, threads = 1; - zend_long memory_cost = PHP_SODIUM_PWHASH_MEMLIMIT << 10; + zend_long memory_cost = PHP_SODIUM_PWHASH_MEMLIMIT; zend_long time_cost = PHP_SODIUM_PWHASH_OPSLIMIT; if (!hash || (ZSTR_LEN(hash) < sizeof("$argon2id$"))) {