]> granicus.if.org Git - php/commitdiff
Promote warnings to exceptions in password_*() functions
authorMáté Kocsis <kocsismate@woohoolabs.com>
Wed, 20 Nov 2019 01:21:06 +0000 (02:21 +0100)
committerMáté Kocsis <kocsismate@woohoolabs.com>
Thu, 12 Dec 2019 11:14:53 +0000 (12:14 +0100)
ext/sodium/sodium_pwhash.c
ext/standard/basic_functions.stub.php
ext/standard/basic_functions_arginfo.h
ext/standard/password.c
ext/standard/tests/password/password_bcrypt_errors.phpt
ext/standard/tests/password/password_hash_error.phpt
ext/standard/tests/password/password_hash_error_argon2.phpt

index 3ff304c452920821d816068bca8a8e8094005f22..e58a9514cc10a14711deadefb307cd8b36bd96aa 100644 (file)
@@ -50,7 +50,7 @@ static inline int get_options(zend_array *options, size_t *memlimit, size_t *ops
                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");
+                       zend_value_error("Memory cost is outside of allowed memory range");
                        return FAILURE;
                }
                *memlimit = smemlimit << 10;
@@ -58,12 +58,12 @@ static inline int get_options(zend_array *options, size_t *memlimit, size_t *ops
        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");
+                       zend_value_error("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");
+               zend_value_error("A thread value other than 1 is not supported by this implementation");
                return FAILURE;
        }
        return SUCCESS;
@@ -74,7 +74,7 @@ static zend_string *php_sodium_argon2_hash(const zend_string *password, zend_arr
        zend_string *ret;
 
        if ((ZSTR_LEN(password) >= 0xffffffff)) {
-               php_error_docref(NULL, E_WARNING, "Password is too long");
+               zend_value_error("Password is too long");
                return NULL;
        }
 
@@ -84,7 +84,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, alg)) {
-               php_error_docref(NULL, E_WARNING, "Unexpected failure hashing password");
+               zend_value_error("Unexpected failure hashing password");
                zend_string_release(ret);
                return NULL;
        }
index 75eeaffedfcd337d89e30a1d16e4b4c7c2c41fca..d05b155dab8033aac7db22a2d9cd4760b32afb4e 100755 (executable)
@@ -1131,7 +1131,7 @@ function unpack(string $format, string $data, int $offset = 0): array|false {}
 
 function password_get_info(string $hash): ?array {}
 
-function password_hash(string $password, $algo, array $options = []): ?string {}
+function password_hash(string $password, $algo, array $options = []): string {}
 
 function password_needs_rehash(string $hash, $algo, array $options = []): bool {}
 
index 0ffd3e7ee65ab189e8895c5a381056c5dca11240..2fa47cb7041400b6b295acf33c2471fcd89e9ffd 100755 (executable)
@@ -1741,7 +1741,7 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_password_get_info, 0, 1, IS_ARRA
        ZEND_ARG_TYPE_INFO(0, hash, IS_STRING, 0)
 ZEND_END_ARG_INFO()
 
-ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_password_hash, 0, 2, IS_STRING, 1)
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_password_hash, 0, 2, IS_STRING, 0)
        ZEND_ARG_TYPE_INFO(0, password, IS_STRING, 0)
        ZEND_ARG_INFO(0, algo)
        ZEND_ARG_TYPE_INFO(0, options, IS_ARRAY, 0)
index 17896e77ee6fa03eb438ff9ff97ec0f616bf229a..56908b4f8ae71c765c7de95fc61b24bc077c03fb 100644 (file)
@@ -83,20 +83,20 @@ static zend_string* php_password_make_salt(size_t length) /* {{{ */
        zend_string *ret, *buffer;
 
        if (length > (INT_MAX / 3)) {
-               php_error_docref(NULL, E_WARNING, "Length is too large to safely generate");
+               zend_value_error("Length is too large to safely generate");
                return NULL;
        }
 
        buffer = zend_string_alloc(length * 3 / 4 + 1, 0);
        if (FAILURE == php_random_bytes_silent(ZSTR_VAL(buffer), ZSTR_LEN(buffer))) {
-               php_error_docref(NULL, E_WARNING, "Unable to generate salt");
+               zend_value_error("Unable to generate salt");
                zend_string_release_ex(buffer, 0);
                return NULL;
        }
 
        ret = zend_string_alloc(length, 0);
        if (php_password_salt_to64(ZSTR_VAL(buffer), ZSTR_LEN(buffer), length, ZSTR_VAL(ret)) == FAILURE) {
-               php_error_docref(NULL, E_WARNING, "Generated salt too short");
+               zend_value_error("Generated salt too short");
                zend_string_release_ex(buffer, 0);
                zend_string_release_ex(ret, 0);
                return NULL;
@@ -193,7 +193,7 @@ static zend_string* php_password_bcrypt_hash(const zend_string *password, zend_a
        }
 
        if (cost < 4 || cost > 31) {
-               php_error_docref(NULL, E_WARNING, "Invalid bcrypt cost parameter specified: " ZEND_LONG_FMT, cost);
+               zend_value_error("Invalid bcrypt cost parameter specified: " ZEND_LONG_FMT, cost);
                return NULL;
        }
 
@@ -316,7 +316,7 @@ static zend_string *php_password_argon2_hash(const zend_string *password, zend_a
        }
 
        if (memory_cost > ARGON2_MAX_MEMORY || memory_cost < ARGON2_MIN_MEMORY) {
-               php_error_docref(NULL, E_WARNING, "Memory cost is outside of allowed memory range");
+               zend_value_error("Memory cost is outside of allowed memory range");
                return NULL;
        }
 
@@ -325,7 +325,7 @@ static zend_string *php_password_argon2_hash(const zend_string *password, zend_a
        }
 
        if (time_cost > ARGON2_MAX_TIME || time_cost < ARGON2_MIN_TIME) {
-               php_error_docref(NULL, E_WARNING, "Time cost is outside of allowed time range");
+               zend_value_error("Time cost is outside of allowed time range");
                return NULL;
        }
 
@@ -334,7 +334,7 @@ static zend_string *php_password_argon2_hash(const zend_string *password, zend_a
        }
 
        if (threads > ARGON2_MAX_LANES || threads == 0) {
-               php_error_docref(NULL, E_WARNING, "Invalid number of threads");
+               zend_value_error("Invalid number of threads");
                return NULL;
        }
 
@@ -374,7 +374,7 @@ static zend_string *php_password_argon2_hash(const zend_string *password, zend_a
 
        if (status != ARGON2_OK) {
                zend_string_efree(encoded);
-               php_error_docref(NULL, E_WARNING, "%s", argon2_error_message(status));
+               zend_value_error("%s", argon2_error_message(status));
                return NULL;
        }
 
@@ -650,7 +650,7 @@ PHP_FUNCTION(password_verify)
 }
 /* }}} */
 
-/* {{{ proto string|null password_hash(string password, mixed algo[, array options = array()])
+/* {{{ proto string password_hash(string password, mixed algo[, array options = array()])
 Hash a password */
 PHP_FUNCTION(password_hash)
 {
@@ -669,15 +669,17 @@ PHP_FUNCTION(password_hash)
        algo = php_password_algo_find_zval(zalgo);
        if (!algo) {
                zend_string *algostr = zval_get_string(zalgo);
-               php_error_docref(NULL, E_WARNING, "Unknown password hashing algorithm: %s", ZSTR_VAL(algostr));
+               zend_value_error("Unknown password hashing algorithm: %s", ZSTR_VAL(algostr));
                zend_string_release(algostr);
-               RETURN_NULL();
+               return;
        }
 
        digest = algo->hash(password, options);
        if (!digest) {
-               /* algo->hash should have raised an error. */
-               RETURN_NULL();
+               if (!EG(exception)) {
+                       zend_throw_error(NULL, "Password hashing failed for unknown reason");
+               }
+               return;
        }
 
        RETURN_NEW_STR(digest);
index 64496744cb1a97bef42e3d2216db742642a55394..10c3483f5a80d0c1d67a0144d93d97f205672851 100644 (file)
@@ -3,15 +3,18 @@ Test error operation of password_hash() with bcrypt hashing
 --FILE--
 <?php
 //-=-=-=-
+try {
+    password_hash("foo", PASSWORD_BCRYPT, array("cost" => 3));
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
 
-var_dump(password_hash("foo", PASSWORD_BCRYPT, array("cost" => 3)));
-
-var_dump(password_hash("foo", PASSWORD_BCRYPT, array("cost" => 32)));
-
+try {
+    var_dump(password_hash("foo", PASSWORD_BCRYPT, array("cost" => 32)));
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
 ?>
---EXPECTF--
-Warning: password_hash(): Invalid bcrypt cost parameter specified: 3 in %s on line %d
-NULL
-
-Warning: password_hash(): Invalid bcrypt cost parameter specified: 32 in %s on line %d
-NULL
+--EXPECT--
+Invalid bcrypt cost parameter specified: 3
+Invalid bcrypt cost parameter specified: 32
index cb5065490590716aa7b566a736b8eecb472b18ff..a0d8c6461e71fc3ec9c746aa2c71cb02a277a95d 100644 (file)
@@ -10,7 +10,11 @@ try {
     echo $e->getMessage(), "\n";
 }
 
-var_dump(password_hash("foo", array()));
+try {
+    password_hash("foo", array());
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
 
 try {
     var_dump(password_hash("foo", 19, new StdClass));
@@ -35,9 +39,7 @@ try {
 password_hash() expects at least 2 parameters, 1 given
 
 Warning: Array to string conversion in %s on line %d
-
-Warning: password_hash(): Unknown password hashing algorithm: Array in %s on line %d
-NULL
+Unknown password hashing algorithm: Array
 password_hash() expects parameter 3 to be array, object given
 password_hash() expects parameter 3 to be array, string given
 password_hash() expects parameter 1 to be string, array given
index 2ea6b93627c0f86f1fb85578bbcdf13e19647dbc..96f029aba93d68ff7272070765f62b569034da69 100644 (file)
@@ -7,28 +7,46 @@ if (!defined('PASSWORD_ARGON2ID')) die('skip password_hash not built with Argon2
 ?>
 --FILE--
 <?php
-var_dump(password_hash('test', PASSWORD_ARGON2I, ['memory_cost' => 0]));
-var_dump(password_hash('test', PASSWORD_ARGON2I, ['time_cost' => 0]));
-var_dump(password_hash('test', PASSWORD_ARGON2I, ['threads' => 0]));
-var_dump(password_hash('test', PASSWORD_ARGON2ID, ['memory_cost' => 0]));
-var_dump(password_hash('test', PASSWORD_ARGON2ID, ['time_cost' => 0]));
-var_dump(password_hash('test', PASSWORD_ARGON2ID, ['threads' => 0]));
-?>
---EXPECTF--
-Warning: password_hash(): Memory cost is outside of allowed memory range in %s on line %d
-NULL
+try {
+    password_hash('test', PASSWORD_ARGON2I, ['memory_cost' => 0]);
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
 
-Warning: password_hash(): Time cost is outside of allowed time range in %s on line %d
-NULL
+try {
+    password_hash('test', PASSWORD_ARGON2I, ['time_cost' => 0]);
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
 
-Warning: password_hash(): %sthread%s
-NULL
+try {
+    password_hash('test', PASSWORD_ARGON2I, ['threads' => 0]);
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
 
-Warning: password_hash(): Memory cost is outside of allowed memory range in %s on line %d
-NULL
+try {
+    password_hash('test', PASSWORD_ARGON2ID, ['memory_cost' => 0]);
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
 
-Warning: password_hash(): Time cost is outside of allowed time range in %s on line %d
-NULL
+try {
+    password_hash('test', PASSWORD_ARGON2ID, ['time_cost' => 0]);
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
 
-Warning: password_hash(): %sthread%s
-NULL
+try {
+    password_hash('test', PASSWORD_ARGON2ID, ['threads' => 0]);
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
+?>
+--EXPECT--
+Memory cost is outside of allowed memory range
+Time cost is outside of allowed time range
+Invalid number of threads
+Memory cost is outside of allowed memory range
+Time cost is outside of allowed time range
+Invalid number of threads