Refactor password.c

Use zend_string instread of char*/size_t
Clean up use of scope vars
Get rid of some temporaries/overstacked expressions.

1 files 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;