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 dfec98cd4ca6dbc97ad887345b58363f13d87047..744c8f8754caedac29123d39b5e65bf1e74b4c67 100644 (file)
--- 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 7b7f574e173a9138c2b78c872f6e306560fb6000..2b284c711685abc76be17443352202d56c7aabef 100644 (file)
--- 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 (file)
index 0000000..524b233
--- /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--
+<?php
+if (!extension_loaded('sodium')) die('skip sodium extension not available');
+?>
+--FILE--
+<?php
+$pass = password_hash('secret', PASSWORD_ARGON2ID, ['memory_cost' => 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)