From 7165e28738ee4393e6e09e8df6a9ae74c25389d3 Mon Sep 17 00:00:00 2001 From: Sara Golemon Date: Sat, 15 Jul 2017 10:12:20 -0400 Subject: [PATCH] Refactor password.c Use zend_string instread of char*/size_t Clean up use of scope vars Get rid of some temporaries/overstacked expressions. --- ext/standard/password.c | 196 ++++++++++++++++++---------------------- 1 file changed, 86 insertions(+), 110 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index 5e1704fe69..41613f65f6 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -58,28 +58,30 @@ PHP_MINIT_FUNCTION(password) /* {{{ */ } /* }}} */ -static char* php_password_get_algo_name(const php_password_algo algo) +static zend_string* php_password_get_algo_name(const php_password_algo algo) { switch (algo) { case PHP_PASSWORD_BCRYPT: - return "bcrypt"; + return zend_string_init("bcrypt", sizeof("bcrypt") - 1, 0); #if HAVE_ARGON2LIB case PHP_PASSWORD_ARGON2I: - return "argon2i"; + return zend_string_init("argon2i", sizeof("argon2i") - 1, 0); #endif case PHP_PASSWORD_UNKNOWN: default: - return "unknown"; + return zend_string_init("unknown", sizeof("unknown") - 1, 0); } } -static php_password_algo php_password_determine_algo(const char *hash, const size_t len) +static php_password_algo php_password_determine_algo(const zend_string *hash) { - if (len > 3 && hash[0] == '$' && hash[1] == '2' && hash[2] == 'y' && len == 60) { + const char *h = ZSTR_VAL(hash); + const size_t len = ZSTR_LEN(hash); + if (len == 60 && h[0] == '$' && h[1] == '2' && h[2] == 'y') { return PHP_PASSWORD_BCRYPT; } #if HAVE_ARGON2LIB - if (len >= sizeof("$argon2i$")-1 && !memcmp(hash, "$argon2i$", sizeof("$argon2i$")-1)) { + if (len >= sizeof("$argon2i$")-1 && !memcmp(h, "$argon2i$", sizeof("$argon2i$")-1)) { return PHP_PASSWORD_ARGON2I; } #endif @@ -128,39 +130,32 @@ static int php_password_salt_to64(const char *str, const size_t str_len, const s } /* }}} */ -static int php_password_make_salt(size_t length, char *ret) /* {{{ */ +static zend_string* php_password_make_salt(size_t length) /* {{{ */ { - size_t raw_length; - char *buffer; - char *result; + zend_string *ret, *buffer; if (length > (INT_MAX / 3)) { - php_error_docref(NULL, E_WARNING, "Length is too large to safely generate"); - return FAILURE; + php_error(E_WARNING, "Length is too large to safely generate"); + return NULL; } - raw_length = length * 3 / 4 + 1; - - buffer = (char *) safe_emalloc(raw_length, 1, 1); - - if (FAILURE == php_random_bytes_silent(buffer, raw_length)) { - php_error_docref(NULL, E_WARNING, "Unable to generate salt"); - efree(buffer); - return FAILURE; + buffer = zend_string_alloc(length * 3 / 4 + 1, 0); + if (FAILURE == php_random_bytes_silent(ZSTR_VAL(buffer), ZSTR_LEN(buffer))) { + php_error(E_WARNING, "Unable to generate salt"); + zend_string_release(buffer); + return NULL; } - result = safe_emalloc(length, 1, 1); - if (php_password_salt_to64(buffer, raw_length, length, result) == FAILURE) { - php_error_docref(NULL, E_WARNING, "Generated salt too short"); - efree(buffer); - efree(result); - return FAILURE; + ret = zend_string_alloc(length, 0); + if (php_password_salt_to64(ZSTR_VAL(buffer), ZSTR_LEN(buffer), length, ZSTR_VAL(ret)) == FAILURE) { + php_error(E_WARNING, "Generated salt too short"); + zend_string_release(buffer); + zend_string_release(ret); + return NULL; } - memcpy(ret, result, length); - efree(result); - efree(buffer); - ret[length] = 0; - return SUCCESS; + zend_string_release(buffer); + ZSTR_VAL(ret)[length] = 0; + return ret; } /* }}} */ @@ -169,24 +164,23 @@ Retrieves information about a given hash */ PHP_FUNCTION(password_get_info) { php_password_algo algo; - size_t hash_len; - char *hash, *algo_name; + zend_string *hash, *algo_name; zval options; ZEND_PARSE_PARAMETERS_START(1, 1) - Z_PARAM_STRING(hash, hash_len) + Z_PARAM_STR(hash) ZEND_PARSE_PARAMETERS_END(); array_init(&options); - algo = php_password_determine_algo(hash, (size_t) hash_len); + algo = php_password_determine_algo(hash); algo_name = php_password_get_algo_name(algo); switch (algo) { case PHP_PASSWORD_BCRYPT: { zend_long cost = PHP_PASSWORD_BCRYPT_COST; - sscanf(hash, "$2y$" ZEND_LONG_FMT "$", &cost); + sscanf(ZSTR_VAL(hash), "$2y$" ZEND_LONG_FMT "$", &cost); add_assoc_long(&options, "cost", cost); } break; @@ -198,7 +192,7 @@ PHP_FUNCTION(password_get_info) zend_long time_cost = PHP_PASSWORD_ARGON2_TIME_COST; zend_long threads = PHP_PASSWORD_ARGON2_THREADS; - sscanf(hash, "$%*[argon2i]$v=" ZEND_LONG_FMT "$m=" ZEND_LONG_FMT ",t=" ZEND_LONG_FMT ",p=" ZEND_LONG_FMT, &v, &memory_cost, &time_cost, &threads); + sscanf(ZSTR_VAL(hash), "$%*[argon2i]$v=" ZEND_LONG_FMT "$m=" ZEND_LONG_FMT ",t=" ZEND_LONG_FMT ",p=" ZEND_LONG_FMT, &v, &memory_cost, &time_cost, &threads); add_assoc_long(&options, "memory_cost", memory_cost); add_assoc_long(&options, "time_cost", time_cost); add_assoc_long(&options, "threads", threads); @@ -213,7 +207,7 @@ PHP_FUNCTION(password_get_info) array_init(return_value); add_assoc_long(return_value, "algo", algo); - add_assoc_string(return_value, "algoName", algo_name); + add_assoc_str(return_value, "algoName", algo_name); add_assoc_zval(return_value, "options", &options); } /** }}} */ @@ -224,19 +218,18 @@ PHP_FUNCTION(password_needs_rehash) { zend_long new_algo = 0; php_password_algo algo; - size_t hash_len; - char *hash; + zend_string *hash; HashTable *options = 0; zval *option_buffer; ZEND_PARSE_PARAMETERS_START(2, 3) - Z_PARAM_STRING(hash, hash_len) + Z_PARAM_STR(hash) Z_PARAM_LONG(new_algo) Z_PARAM_OPTIONAL Z_PARAM_ARRAY_OR_OBJECT_HT(options) ZEND_PARSE_PARAMETERS_END(); - algo = php_password_determine_algo(hash, (size_t) hash_len); + algo = php_password_determine_algo(hash); if ((zend_long)algo != new_algo) { RETURN_TRUE; @@ -251,7 +244,7 @@ PHP_FUNCTION(password_needs_rehash) new_cost = zval_get_long(option_buffer); } - sscanf(hash, "$2y$" ZEND_LONG_FMT "$", &cost); + sscanf(ZSTR_VAL(hash), "$2y$" ZEND_LONG_FMT "$", &cost); if (cost != new_cost) { RETURN_TRUE; } @@ -277,7 +270,7 @@ PHP_FUNCTION(password_needs_rehash) new_threads = zval_get_long(option_buffer); } - sscanf(hash, "$%*[argon2i]$v=" ZEND_LONG_FMT "$m=" ZEND_LONG_FMT ",t=" ZEND_LONG_FMT ",p=" ZEND_LONG_FMT, &v, &memory_cost, &time_cost, &threads); + sscanf(ZSTR_VAL(hash), "$%*[argon2i]$v=" ZEND_LONG_FMT "$m=" ZEND_LONG_FMT ",t=" ZEND_LONG_FMT ",p=" ZEND_LONG_FMT, &v, &memory_cost, &time_cost, &threads); if (new_time_cost != time_cost || new_memory_cost != memory_cost || new_threads != threads) { RETURN_TRUE; @@ -297,44 +290,35 @@ PHP_FUNCTION(password_needs_rehash) Verify a hash created using crypt() or password_hash() */ PHP_FUNCTION(password_verify) { - int status = 0; - size_t i, password_len, hash_len; - char *password, *hash; - zend_string *ret; + zend_string *password, *hash; php_password_algo algo; ZEND_PARSE_PARAMETERS_START(2, 2) - Z_PARAM_STRING(password, password_len) - Z_PARAM_STRING(hash, hash_len) + Z_PARAM_STR(password) + Z_PARAM_STR(hash) ZEND_PARSE_PARAMETERS_END_EX(RETURN_FALSE); - algo = php_password_determine_algo(hash, (size_t) hash_len); + algo = php_password_determine_algo(hash); switch(algo) { #if HAVE_ARGON2LIB case PHP_PASSWORD_ARGON2I: - { - argon2_type type = Argon2_i; - - status = argon2_verify(hash, password, password_len, type); - - if (status == ARGON2_OK) { - RETURN_TRUE; - } - - RETURN_FALSE; - } + RETURN_BOOL(ARGON2_OK == argon2_verify(ZSTR_VAL(hash), ZSTR_VAL(password), ZSTR_LEN(password), Argon2_i)); break; #endif case PHP_PASSWORD_BCRYPT: case PHP_PASSWORD_UNKNOWN: default: { - if ((ret = php_crypt(password, (int)password_len, hash, (int)hash_len, 1)) == NULL) { + size_t i; + int status = 0; + zend_string *ret = php_crypt(ZSTR_VAL(password), (int)ZSTR_LEN(password), ZSTR_VAL(hash), (int)ZSTR_LEN(hash), 1); + + if (!ret) { RETURN_FALSE; } - if (ZSTR_LEN(ret) != hash_len || hash_len < 13) { + if (ZSTR_LEN(ret) != ZSTR_LEN(hash) || ZSTR_LEN(hash) < 13) { zend_string_free(ret); RETURN_FALSE; } @@ -343,8 +327,8 @@ PHP_FUNCTION(password_verify) * resistance towards timing attacks. This is a constant time * equality check that will always check every byte of both * values. */ - for (i = 0; i < hash_len; i++) { - status |= (ZSTR_VAL(ret)[i] ^ hash[i]); + for (i = 0; i < ZSTR_LEN(hash); i++) { + status |= (ZSTR_VAL(ret)[i] ^ ZSTR_VAL(hash)[i]); } zend_string_free(ret); @@ -361,11 +345,10 @@ PHP_FUNCTION(password_verify) Hash a password */ PHP_FUNCTION(password_hash) { - char hash_format[8], *hash, *salt, *password; + char hash_format[8]; zend_long algo = 0; - size_t password_len = 0; - int hash_len; - size_t salt_len = 0, required_salt_len = 0, hash_format_len; + zend_string *password, *salt; + size_t required_salt_len = 0, hash_format_len; HashTable *options = 0; zval *option_buffer; @@ -377,7 +360,7 @@ PHP_FUNCTION(password_hash) #endif ZEND_PARSE_PARAMETERS_START(2, 3) - Z_PARAM_STRING(password, password_len) + Z_PARAM_STR(password) Z_PARAM_LONG(algo) Z_PARAM_OPTIONAL Z_PARAM_ARRAY_OR_OBJECT_HT(options) @@ -477,51 +460,45 @@ PHP_FUNCTION(password_hash) zend_string_release(buffer); RETURN_NULL(); } else if (php_password_salt_is_alphabet(ZSTR_VAL(buffer), ZSTR_LEN(buffer)) == FAILURE) { - salt = safe_emalloc(required_salt_len, 1, 1); - if (php_password_salt_to64(ZSTR_VAL(buffer), ZSTR_LEN(buffer), required_salt_len, salt) == FAILURE) { - efree(salt); + salt = zend_string_alloc(required_salt_len, 0); + if (php_password_salt_to64(ZSTR_VAL(buffer), ZSTR_LEN(buffer), required_salt_len, ZSTR_VAL(salt)) == FAILURE) { + zend_string_release(salt); php_error_docref(NULL, E_WARNING, "Provided salt is too short: %zd", ZSTR_LEN(buffer)); zend_string_release(buffer); RETURN_NULL(); } - salt_len = required_salt_len; } else { - salt = safe_emalloc(required_salt_len, 1, 1); - memcpy(salt, ZSTR_VAL(buffer), required_salt_len); - salt_len = required_salt_len; + salt = zend_string_alloc(required_salt_len, 0); + memcpy(ZSTR_VAL(salt), ZSTR_VAL(buffer), required_salt_len); } zend_string_release(buffer); } else { - salt = safe_emalloc(required_salt_len, 1, 1); - if (php_password_make_salt(required_salt_len, salt) == FAILURE) { - efree(salt); + salt = php_password_make_salt(required_salt_len); + if (!salt) { RETURN_FALSE; } - salt_len = required_salt_len; } switch (algo) { case PHP_PASSWORD_BCRYPT: { - zend_string *result; - salt[salt_len] = 0; + zend_string *result, *hash; + ZSTR_VAL(salt)[ZSTR_LEN(salt)] = 0; - hash = safe_emalloc(salt_len + hash_format_len, 1, 1); - sprintf(hash, "%s%s", hash_format, salt); - hash[hash_format_len + salt_len] = 0; + hash = zend_string_alloc(ZSTR_LEN(salt) + hash_format_len, 0); + sprintf(ZSTR_VAL(hash), "%s%s", hash_format, ZSTR_VAL(salt)); + ZSTR_VAL(hash)[hash_format_len + ZSTR_LEN(salt)] = 0; - efree(salt); + zend_string_release(salt); /* This cast is safe, since both values are defined here in code and cannot overflow */ - hash_len = (int) (hash_format_len + salt_len); + result = php_crypt(ZSTR_VAL(password), (int)ZSTR_LEN(password), ZSTR_VAL(hash), (int)ZSTR_LEN(hash), 1); + zend_string_release(hash); - if ((result = php_crypt(password, (int)password_len, hash, hash_len, 1)) == NULL) { - efree(hash); + if (!result) { RETURN_FALSE; } - efree(hash); - if (ZSTR_LEN(result) < 13) { zend_string_free(result); RETURN_FALSE; @@ -533,51 +510,50 @@ PHP_FUNCTION(password_hash) #if HAVE_ARGON2LIB case PHP_PASSWORD_ARGON2I: { - size_t out_len = 32; size_t encoded_len; int status = 0; - char *out; + zend_string *out = zend_string_alloc(32, 0); zend_string *encoded; encoded_len = argon2_encodedlen( time_cost, memory_cost, threads, - (uint32_t)salt_len, - out_len + (uint32_t)ZSTR_LEN(salt), + ZSTR_LEN(out) #if HAVE_ARGON2ID , type #endif ); - out = emalloc(out_len + 1); encoded = zend_string_alloc(encoded_len, 0); status = argon2_hash( time_cost, memory_cost, threads, - password, - password_len, - salt, - salt_len, - out, - out_len, + ZSTR_VAL(password), + ZSTR_LEN(password), + ZSTR_VAL(salt), + ZSTR_LEN(salt), + ZSTR_VAL(out), + ZSTR_LEN(out), ZSTR_VAL(encoded), - encoded_len, + ZSTR_LEN(encoded), type, ARGON2_VERSION_NUMBER ); - efree(out); - efree(salt); + zend_string_release(out); + zend_string_release(salt); if (status != ARGON2_OK) { zend_string_free(encoded); - php_error_docref(NULL, E_WARNING, argon2_error_message(status)); + php_error(E_WARNING, "%s", argon2_error_message(status)); RETURN_FALSE; } - + + ZSTR_VAL(encoded)[ZSTR_LEN(encoded)] = 0; RETURN_STR(encoded); } break; -- 2.40.0