From c77f2c29585f97bd9dad533b9d2bc8334de34f1b Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Sun, 24 Jun 2012 22:44:43 -0400 Subject: [PATCH 01/44] Base structure for passsword_create and password_make_salt --- ext/standard/basic_functions.c | 20 +++ ext/standard/config.m4 | 2 +- ext/standard/config.w32 | 2 +- ext/standard/password.c | 257 +++++++++++++++++++++++++++++++++ ext/standard/php_password.h | 48 ++++++ ext/standard/php_standard.h | 1 + 6 files changed, 328 insertions(+), 2 deletions(-) create mode 100644 ext/standard/password.c create mode 100644 ext/standard/php_password.h diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c index 63d40efde4400..64025db625729 100644 --- a/ext/standard/basic_functions.c +++ b/ext/standard/basic_functions.c @@ -1866,6 +1866,21 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_INFO(arginfo_getlastmod, 0) ZEND_END_ARG_INFO() /* }}} */ +/* {{{ password.c */ +ZEND_BEGIN_ARG_INFO_EX(arginfo_password_create, 0, 0, 1) + ZEND_ARG_INFO(0, password) + ZEND_ARG_INFO(0, algo) + ZEND_ARG_INFO(0, options) +ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_INFO_EX(arginfo_password_verify, 0, 0, 2) + ZEND_ARG_INFO(0, password) + ZEND_ARG_INFO(0, hash) +ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_INFO_EX(arginfo_password_make_salt, 0, 0, 1) + ZEND_ARG_INFO(0, length) + ZEND_ARG_INFO(0, raw_output) +ZEND_END_ARG_INFO() +/* }}} */ /* {{{ proc_open.c */ #ifdef PHP_CAN_SUPPORT_PROC_OPEN ZEND_BEGIN_ARG_INFO_EX(arginfo_proc_terminate, 0, 0, 1) @@ -2880,6 +2895,10 @@ const zend_function_entry basic_functions[] = { /* {{{ */ PHP_FE(base64_decode, arginfo_base64_decode) PHP_FE(base64_encode, arginfo_base64_encode) + PHP_FE(password_create, arginfo_password_create) + PHP_FE(password_verify, arginfo_password_verify) + PHP_FE(password_make_salt, arginfo_password_make_salt) + PHP_FE(convert_uuencode, arginfo_convert_uuencode) PHP_FE(convert_uudecode, arginfo_convert_uudecode) @@ -3630,6 +3649,7 @@ PHP_MINIT_FUNCTION(basic) /* {{{ */ BASIC_MINIT_SUBMODULE(browscap) BASIC_MINIT_SUBMODULE(standard_filters) BASIC_MINIT_SUBMODULE(user_filters) + BASIC_MINIT_SUBMODULE(password) #if defined(HAVE_LOCALECONV) && defined(ZTS) BASIC_MINIT_SUBMODULE(localeconv) diff --git a/ext/standard/config.m4 b/ext/standard/config.m4 index c33ae1e05c897..fba423b19195a 100644 --- a/ext/standard/config.m4 +++ b/ext/standard/config.m4 @@ -580,7 +580,7 @@ PHP_NEW_EXTENSION(standard, array.c base64.c basic_functions.c browscap.c crc32. incomplete_class.c url_scanner_ex.c ftp_fopen_wrapper.c \ http_fopen_wrapper.c php_fopen_wrapper.c credits.c css.c \ var_unserializer.c ftok.c sha1.c user_filters.c uuencode.c \ - filters.c proc_open.c streamsfuncs.c http.c) + filters.c proc_open.c streamsfuncs.c http.c password.c) PHP_ADD_MAKEFILE_FRAGMENT PHP_INSTALL_HEADERS([ext/standard/]) diff --git a/ext/standard/config.w32 b/ext/standard/config.w32 index d14b859e9db99..5f24641b4d33a 100644 --- a/ext/standard/config.w32 +++ b/ext/standard/config.w32 @@ -19,7 +19,7 @@ EXTENSION("standard", "array.c base64.c basic_functions.c browscap.c \ versioning.c assert.c strnatcmp.c levenshtein.c incomplete_class.c \ url_scanner_ex.c ftp_fopen_wrapper.c http_fopen_wrapper.c \ php_fopen_wrapper.c credits.c css.c var_unserializer.c ftok.c sha1.c \ - user_filters.c uuencode.c filters.c proc_open.c \ + user_filters.c uuencode.c filters.c proc_open.c password.c \ streamsfuncs.c http.c flock_compat.c", false /* never shared */); PHP_INSTALL_HEADERS("", "ext/standard"); if (PHP_MBREGEX != "no") { diff --git a/ext/standard/password.c b/ext/standard/password.c new file mode 100644 index 0000000000000..677f1325caa2a --- /dev/null +++ b/ext/standard/password.c @@ -0,0 +1,257 @@ +/* + +----------------------------------------------------------------------+ + | PHP Version 5 | + +----------------------------------------------------------------------+ + | Copyright (c) 1997-2012 The PHP Group | + +----------------------------------------------------------------------+ + | This source file is subject to version 3.01 of the PHP license, | + | that is bundled with this package in the file LICENSE, and is | + | available through the world-wide-web at the following url: | + | http://www.php.net/license/3_01.txt | + | If you did not receive a copy of the PHP license and are unable to | + | obtain it through the world-wide-web, please send a note to | + | license@php.net so we can mail you a copy immediately. | + +----------------------------------------------------------------------+ + | Authors: Anthony Ferrara | + +----------------------------------------------------------------------+ +*/ + +/* $Id$ */ + +#include + +#include "php.h" +#if HAVE_CRYPT +#include "php_crypt.h" +#endif + +#include "php_password.h" +#include "php_rand.h" +#include "base64.h" + + +PHP_MINIT_FUNCTION(password) /* {{{ */ +{ + REGISTER_STRING_CONSTANT("PASSWORD_DEFAULT", PHP_PASSWORD_DEFAULT, CONST_CS | CONST_PERSISTENT); + REGISTER_STRING_CONSTANT("PASSWORD_BCRYPT", PHP_PASSWORD_BCRYPT, CONST_CS | CONST_PERSISTENT); + REGISTER_STRING_CONSTANT("PASSWORD_MD5", PHP_PASSWORD_MD5, CONST_CS | CONST_PERSISTENT); + REGISTER_STRING_CONSTANT("PASSWORD_SHA256", PHP_PASSWORD_SHA256, CONST_CS | CONST_PERSISTENT); + REGISTER_STRING_CONSTANT("PASSWORD_SHA512", PHP_PASSWORD_SHA512, CONST_CS | CONST_PERSISTENT); + return SUCCESS; +} +/* }}} */ + + +static int php_password_salt_is_alphabet(const char *str, const int len) +{ + int i = 0; + + for (i = 0; i < len; i++) { + if (!((str[i] >= 'A' && str[i] <= 'Z') || (str[i] >= 'a' && str[i] <= 'z') || (str[i] >= '0' && str[i] <= '9') || str[i] == '.' || str[i] == '/')) { + return 0; + } + } + return 1; +} + +static int php_password_salt_to64(const char *str, const int str_len, const int out_len, char *ret) +{ + int pos = 0; + unsigned char *buffer; + buffer = php_base64_encode((unsigned char*) str, str_len, NULL); + for (pos = 0; pos < out_len; pos++) { + if (buffer[pos] == '+') { + ret[pos] = '.'; + } else if (buffer[pos] == '=') { + efree(buffer); + return FAILURE; + } else { + ret[pos] = buffer[pos]; + } + } + efree(buffer); + return SUCCESS; +} + +static int php_password_make_salt(int length, int raw, char *ret) +{ + int i, raw_length; + char *buffer; + if (raw) { + raw_length = length; + } else { + raw_length = length * 3 / 4 + 1; + } + buffer = (char *) emalloc(raw_length + 1); + + /* Temp Placeholder */ + for (i = 0; i < raw_length; i++) { + buffer[i] = i; + } + /* /Temp Placeholder */ + + if (raw) { + memcpy(ret, buffer, length); + } else { + char *result; + result = emalloc(length + 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; + } else { + memcpy(ret, result, length); + efree(result); + } + } + efree(buffer); + ret[length] = 0; + return SUCCESS; +} + +PHP_FUNCTION(password_verify) +{ +} + +PHP_FUNCTION(password_make_salt) +{ + char *salt; + int length = 0; + zend_bool raw_output = 0; + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "l|b", &length, &raw_output) == FAILURE) { + RETURN_FALSE; + } + if (length <= 0) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Length cannot be less than or equal zero: %d", length); + RETURN_FALSE; + } + salt = emalloc(length + 1); + if (php_password_make_salt(length, (int) raw_output, salt) == FAILURE) { + efree(salt); + RETURN_FALSE; + } + RETURN_STRINGL(salt, length, 0); +} + + +/* {{{ proto string password(string password, string algo = PASSWORD_DEFAULT, array options = array()) +Hash a password */ +PHP_FUNCTION(password_create) +{ + char *password, *algo = 0, *hash_format, *hash, *salt; + int password_len, algo_len = 0, salt_len = 0, required_salt_len = 0, hash_format_len; + HashTable *options = 0; + zval **option_buffer; + + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s|sH", &password, &password_len, &algo, &algo_len, &options) == FAILURE) { + RETURN_FALSE; + } + + if (algo_len == 0) { + algo = PHP_PASSWORD_DEFAULT; + algo_len = strlen(PHP_PASSWORD_DEFAULT); + } + + if (strcmp(algo, PHP_PASSWORD_BCRYPT) == 0) { + int cost = PHP_PASSWORD_BCRYPT_DEFAULT_COST; + if (options && zend_symtable_find(options, "cost", 5, (void **) &option_buffer) == SUCCESS) { + convert_to_long_ex(option_buffer); + cost = Z_LVAL_PP(option_buffer); + zval_ptr_dtor(option_buffer); + if (cost < 4 || cost > 31) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Invalid bcrypt cost parameter specified: %d", cost); + RETURN_FALSE; + } + } + required_salt_len = 22; + hash_format = emalloc(8); + sprintf(hash_format, "$2y$%02d$", cost); + hash_format_len = 7; + } else if (strcmp(algo, PHP_PASSWORD_MD5) == 0) { + required_salt_len = 12; + hash_format = emalloc(4); + memcpy(hash_format, "$1$", 3); + hash_format_len = 3; + } else if (strcmp(algo, PHP_PASSWORD_SHA256) == 0 || strcmp(algo, PHP_PASSWORD_SHA512) == 0) { + int rounds = PHP_PASSWORD_SHA_DEFAULT_ROUNDS; + if (options && zend_symtable_find(options, "rounds", 7, (void **) &option_buffer) == SUCCESS) { + convert_to_long_ex(option_buffer); + rounds = Z_LVAL_PP(option_buffer); + zval_ptr_dtor(option_buffer); + if (rounds < 1000 || rounds > 999999999) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Invalid SHA rounds parameter specified: %d", rounds); + RETURN_FALSE; + } + } + required_salt_len = 16; + hash_format = emalloc(21); + sprintf(hash_format, "$%s$rounds=%d$", algo, rounds); + hash_format_len = strlen(hash_format); + } else { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unknown password hashing algorithm: %s", algo); + RETURN_FALSE; + } + + if (options && zend_symtable_find(options, "salt", 5, (void**) &option_buffer) == SUCCESS) { + char *buffer; + int buffer_len; + if (Z_TYPE_PP(option_buffer) == IS_STRING) { + buffer = Z_STRVAL_PP(option_buffer); + buffer_len = Z_STRLEN_PP(option_buffer); + } else { + zval_ptr_dtor(option_buffer); + efree(hash_format); + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Non-string salt parameter supplied"); + RETURN_FALSE; + } + if (buffer_len < required_salt_len) { + efree(hash_format); + zval_ptr_dtor(option_buffer); + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Provided salt is too short: %d expecting %d", buffer_len, required_salt_len); + RETURN_FALSE; + } else if (0 == php_password_salt_is_alphabet(buffer, buffer_len)) { + salt = emalloc(required_salt_len + 1); + if (php_password_salt_to64(buffer, buffer_len, required_salt_len, salt) == FAILURE) { + efree(hash_format); + zval_ptr_dtor(option_buffer); + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Provided salt is too short: %d", salt_len); + RETURN_FALSE; + } + salt_len = required_salt_len; + } else { + salt = emalloc(required_salt_len + 1); + memcpy(salt, buffer, required_salt_len); + salt_len = required_salt_len; + } + zval_ptr_dtor(option_buffer); + } else { + salt = emalloc(required_salt_len + 1); + if (php_password_make_salt(required_salt_len, 0, salt) == FAILURE) { + efree(hash_format); + efree(salt); + RETURN_FALSE; + } + salt_len = required_salt_len; + } + + salt[salt_len] = 0; + + hash = emalloc(salt_len + hash_format_len + 1); + sprintf(hash, "%s%s", hash_format, salt); + hash[hash_format_len + salt_len] = 0; + efree(hash_format); + efree(salt); + + RETURN_STRINGL(hash, hash_format_len + salt_len, 0); +} +/* }}} */ + +/* + * Local variables: + * tab-width: 4 + * c-basic-offset: 4 + * End: + * vim600: sw=4 ts=4 fdm=marker + * vim<600: sw=4 ts=4 + */ diff --git a/ext/standard/php_password.h b/ext/standard/php_password.h new file mode 100644 index 0000000000000..f813189a46be1 --- /dev/null +++ b/ext/standard/php_password.h @@ -0,0 +1,48 @@ +/* + +----------------------------------------------------------------------+ + | PHP Version 5 | + +----------------------------------------------------------------------+ + | Copyright (c) 1997-2012 The PHP Group | + +----------------------------------------------------------------------+ + | This source file is subject to version 3.01 of the PHP license, | + | that is bundled with this package in the file LICENSE, and is | + | available through the world-wide-web at the following url: | + | http://www.php.net/license/3_01.txt | + | If you did not receive a copy of the PHP license and are unable to | + | obtain it through the world-wide-web, please send a note to | + | license@php.net so we can mail you a copy immediately. | + +----------------------------------------------------------------------+ + | Authors: Anthony Ferrara | + +----------------------------------------------------------------------+ +*/ + +/* $Id$ */ + +#ifndef PHP_PASSWORD_H +#define PHP_PASSWORD_H + +PHP_FUNCTION(password_create); +PHP_FUNCTION(password_verify); +PHP_FUNCTION(password_make_salt); + +PHP_MINIT_FUNCTION(password); + +#define PHP_PASSWORD_DEFAULT "2y" +#define PHP_PASSWORD_BCRYPT "2y" +#define PHP_PASSWORD_MD5 "1" +#define PHP_PASSWORD_SHA256 "5" +#define PHP_PASSWORD_SHA512 "6" + +#define PHP_PASSWORD_BCRYPT_DEFAULT_COST 14; +#define PHP_PASSWORD_SHA_DEFAULT_ROUNDS 5000; + + +#endif + + +/* + * Local variables: + * tab-width: 4 + * c-basic-offset: 4 + * End: + */ diff --git a/ext/standard/php_standard.h b/ext/standard/php_standard.h index 483dbc33bc80a..bccfebe54331f 100644 --- a/ext/standard/php_standard.h +++ b/ext/standard/php_standard.h @@ -58,6 +58,7 @@ #include "php_versioning.h" #include "php_ftok.h" #include "php_type.h" +#include "php_password.h" #define phpext_standard_ptr basic_functions_module_ptr PHP_MINIT_FUNCTION(standard_filters); From 7e41980fe4972e097e178c034f92920c9c63086c Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Sun, 24 Jun 2012 23:25:18 -0400 Subject: [PATCH 02/44] Actually complete password_create() --- ext/standard/password.c | 33 +++++++++++++++++++++++++++------ ext/standard/php_password.h | 2 +- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index 677f1325caa2a..9201ff3f8d1fe 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -28,7 +28,7 @@ #include "php_password.h" #include "php_rand.h" #include "base64.h" - +#include "zend_interfaces.h" PHP_MINIT_FUNCTION(password) /* {{{ */ { @@ -139,15 +139,20 @@ PHP_FUNCTION(password_make_salt) Hash a password */ PHP_FUNCTION(password_create) { - char *password, *algo = 0, *hash_format, *hash, *salt; - int password_len, algo_len = 0, salt_len = 0, required_salt_len = 0, hash_format_len; + char *algo = 0, *hash_format, *hash, *salt; + int algo_len = 0, salt_len = 0, required_salt_len = 0, hash_format_len; HashTable *options = 0; - zval **option_buffer; + zval **option_buffer, *ret, *password, *hash_zval; - if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s|sH", &password, &password_len, &algo, &algo_len, &options) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "z|sH", &password, &algo, &algo_len, &options) == FAILURE) { RETURN_FALSE; } + if (Z_TYPE_P(password) != IS_STRING) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Password must be a string"); + RETURN_FALSE; + } + if (algo_len == 0) { algo = PHP_PASSWORD_DEFAULT; algo_len = strlen(PHP_PASSWORD_DEFAULT); @@ -240,10 +245,26 @@ PHP_FUNCTION(password_create) hash = emalloc(salt_len + hash_format_len + 1); sprintf(hash, "%s%s", hash_format, salt); hash[hash_format_len + salt_len] = 0; + + ALLOC_INIT_ZVAL(hash_zval); + ZVAL_STRINGL(hash_zval, hash, hash_format_len + salt_len, 0); + efree(hash_format); efree(salt); - RETURN_STRINGL(hash, hash_format_len + salt_len, 0); + zend_call_method_with_2_params(NULL, NULL, NULL, "crypt", &ret, password, hash_zval); + + zval_ptr_dtor(&hash_zval); + + if (Z_TYPE_P(ret) != IS_STRING) { + zval_ptr_dtor(&ret); + RETURN_FALSE; + } else if(Z_STRLEN_P(ret) < 13) { + zval_ptr_dtor(&ret); + RETURN_FALSE; + } + + RETURN_ZVAL(ret, 0, 1); } /* }}} */ diff --git a/ext/standard/php_password.h b/ext/standard/php_password.h index f813189a46be1..5967840d51dae 100644 --- a/ext/standard/php_password.h +++ b/ext/standard/php_password.h @@ -33,7 +33,7 @@ PHP_MINIT_FUNCTION(password); #define PHP_PASSWORD_SHA256 "5" #define PHP_PASSWORD_SHA512 "6" -#define PHP_PASSWORD_BCRYPT_DEFAULT_COST 14; +#define PHP_PASSWORD_BCRYPT_DEFAULT_COST 12; #define PHP_PASSWORD_SHA_DEFAULT_ROUNDS 5000; From 657402832b7884f52bf07b2e6f704510395fd413 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Sun, 24 Jun 2012 23:35:26 -0400 Subject: [PATCH 03/44] Implement password_verify --- ext/standard/password.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/ext/standard/password.c b/ext/standard/password.c index 9201ff3f8d1fe..665e69f28ce3b 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -112,6 +112,33 @@ static int php_password_make_salt(int length, int raw, char *ret) PHP_FUNCTION(password_verify) { + zval *password, *hash, *ret; + int status = 0, i; + + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "zz", &password, &hash) == FAILURE) { + RETURN_FALSE; + } + + zend_call_method_with_2_params(NULL, NULL, NULL, "crypt", &ret, password, hash); + + if (Z_TYPE_P(ret) != IS_STRING) { + zval_ptr_dtor(&ret); + RETURN_FALSE; + } + + if (Z_STRLEN_P(ret) != Z_STRLEN_P(hash)) { + zval_ptr_dtor(&ret); + RETURN_FALSE; + } + + for (i = 0; i < Z_STRLEN_P(ret); i++) { + status |= (Z_STRVAL_P(ret)[i] ^ Z_STRVAL_P(hash)[i]); + } + + zval_ptr_dtor(&ret); + + RETURN_BOOL(status == 0); + } PHP_FUNCTION(password_make_salt) From f7097d99ffedc6bd0965542454b4ac86e4b5c914 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Sun, 24 Jun 2012 23:36:09 -0400 Subject: [PATCH 04/44] Fix memory leak on branch --- ext/standard/password.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ext/standard/password.c b/ext/standard/password.c index 665e69f28ce3b..2b7e7dfc9378a 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -246,6 +246,7 @@ PHP_FUNCTION(password_create) salt = emalloc(required_salt_len + 1); if (php_password_salt_to64(buffer, buffer_len, required_salt_len, salt) == FAILURE) { efree(hash_format); + efree(salt); zval_ptr_dtor(option_buffer); php_error_docref(NULL TSRMLS_CC, E_WARNING, "Provided salt is too short: %d", salt_len); RETURN_FALSE; From 18d3bd9481c470d241c492eb39a93bd071a77c4e Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Mon, 25 Jun 2012 08:15:17 -0400 Subject: [PATCH 05/44] Basic random generator added to make_salt --- ext/standard/password.c | 37 +++++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index 2b7e7dfc9378a..f6d8048dac84c 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -25,6 +25,7 @@ #include "php_crypt.h" #endif +#include "ext/hash/php_hash.h" #include "php_password.h" #include "php_rand.h" #include "base64.h" @@ -73,10 +74,14 @@ static int php_password_salt_to64(const char *str, const int str_len, const int return SUCCESS; } -static int php_password_make_salt(int length, int raw, char *ret) +#define PHP_PASSWORD_FUNCTION_EXISTS(func, func_len) (zend_hash_find(EG(function_table), (func), (func_len) + 1, (void **) &func_ptr) == SUCCESS && func_ptr->type == ZEND_INTERNAL_FUNCTION && func_ptr->internal_function.handler != zif_display_disabled_function) + +static int php_password_make_salt(int length, int raw, char *ret TSRMLS_DC) { - int i, raw_length; + int i, raw_length, buffer_valid = 0; char *buffer; + zend_function *func_ptr; + if (raw) { raw_length = length; } else { @@ -85,8 +90,28 @@ static int php_password_make_salt(int length, int raw, char *ret) buffer = (char *) emalloc(raw_length + 1); /* Temp Placeholder */ - for (i = 0; i < raw_length; i++) { - buffer[i] = i; + if (PHP_PASSWORD_FUNCTION_EXISTS("mcrypt_create_iv", 16)) { + zval *ret, *size, *source; + ALLOC_INIT_ZVAL(size); + ZVAL_LONG(size, raw_length); + ALLOC_INIT_ZVAL(source) + ZVAL_LONG(source, 1); // MCRYPT_DEV_URANDOM + zend_call_method_with_2_params(NULL, NULL, NULL, "mcrypt_create_iv", &ret, size, source); + zval_ptr_dtor(&size); + zval_ptr_dtor(&source); + if (Z_TYPE_P(ret) == IS_STRING) { + memcpy(buffer, Z_STRVAL_P(ret), Z_STRLEN_P(ret)); + buffer_valid = 1; + } + zval_ptr_dtor(&ret); + } + if (!buffer_valid) { + long number; + for (i = 0; i < raw_length; i++) { + number = php_rand(TSRMLS_C); + RAND_RANGE(number, 0, 255, PHP_RAND_MAX); + buffer[i] = (char) number; + } } /* /Temp Placeholder */ @@ -154,7 +179,7 @@ PHP_FUNCTION(password_make_salt) RETURN_FALSE; } salt = emalloc(length + 1); - if (php_password_make_salt(length, (int) raw_output, salt) == FAILURE) { + if (php_password_make_salt(length, (int) raw_output, salt TSRMLS_CC) == FAILURE) { efree(salt); RETURN_FALSE; } @@ -260,7 +285,7 @@ PHP_FUNCTION(password_create) zval_ptr_dtor(option_buffer); } else { salt = emalloc(required_salt_len + 1); - if (php_password_make_salt(required_salt_len, 0, salt) == FAILURE) { + if (php_password_make_salt(required_salt_len, 0, salt TSRMLS_CC) == FAILURE) { efree(hash_format); efree(salt); RETURN_FALSE; From 618f2629567ca3a3d1817ca9c4c62339fb5fb886 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Mon, 25 Jun 2012 08:50:39 -0400 Subject: [PATCH 06/44] More error checking, and some cleaning up for password.c --- ext/standard/password.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index f6d8048dac84c..013dab7c6c5e4 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -21,10 +21,6 @@ #include #include "php.h" -#if HAVE_CRYPT -#include "php_crypt.h" -#endif - #include "ext/hash/php_hash.h" #include "php_password.h" #include "php_rand.h" @@ -121,7 +117,7 @@ static int php_password_make_salt(int length, int raw, char *ret TSRMLS_DC) char *result; result = emalloc(length + 1); if (php_password_salt_to64(buffer, raw_length, length, result) == FAILURE) { - php_error_docref(NULL, E_WARNING, "Generated salt too short"); + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Generated salt too short"); efree(buffer); efree(result); return FAILURE; @@ -139,6 +135,12 @@ PHP_FUNCTION(password_verify) { zval *password, *hash, *ret; int status = 0, i; + zend_function *func_ptr; + + if (!PHP_PASSWORD_FUNCTION_EXISTS("crypt", 5)) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Crypt must be loaded for password_verify to function"); + RETURN_FALSE; + } if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "zz", &password, &hash) == FAILURE) { RETURN_FALSE; @@ -195,6 +197,12 @@ PHP_FUNCTION(password_create) int algo_len = 0, salt_len = 0, required_salt_len = 0, hash_format_len; HashTable *options = 0; zval **option_buffer, *ret, *password, *hash_zval; + zend_function *func_ptr; + + if (!PHP_PASSWORD_FUNCTION_EXISTS("crypt", 5)) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Crypt must be loaded for password_verify to function"); + RETURN_FALSE; + } if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "z|sH", &password, &algo, &algo_len, &options) == FAILURE) { RETURN_FALSE; From 41d7374ea4598000fd626c0d8cd4736aec6357bf Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Mon, 25 Jun 2012 11:37:48 -0400 Subject: [PATCH 07/44] Implement openssl support for make_salt --- ext/standard/password.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index 013dab7c6c5e4..f2c94fb473b5c 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -96,11 +96,24 @@ static int php_password_make_salt(int length, int raw, char *ret TSRMLS_DC) zval_ptr_dtor(&size); zval_ptr_dtor(&source); if (Z_TYPE_P(ret) == IS_STRING) { - memcpy(buffer, Z_STRVAL_P(ret), Z_STRLEN_P(ret)); + memcpy(buffer, Z_STRVAL_P(ret), raw_length); buffer_valid = 1; } zval_ptr_dtor(&ret); } + if (!buffer_valid && PHP_PASSWORD_FUNCTION_EXISTS("openssl_random_pseudo_bytes", 27)) { + zval *ret, *size; + ALLOC_INIT_ZVAL(size); + ZVAL_LONG(size, raw_length); + zend_call_method_with_1_params(NULL, NULL, NULL, "openssl_random_pseudo_bytes", &ret, size); + zval_ptr_dtor(&size); + if (Z_TYPE_P(ret) == IS_STRING) { + memcpy(buffer, Z_STRVAL_P(ret), raw_length); + buffer_valid = 1; + } + zval_ptr_dtor(&ret); + } + if (!buffer_valid) { long number; for (i = 0; i < raw_length; i++) { From 2d4b7cb653efc3f52ca907f48b1c828632df5e41 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Mon, 25 Jun 2012 21:22:16 -0400 Subject: [PATCH 08/44] Refactor salt generation, rename password_create to password_hash --- ext/standard/basic_functions.c | 4 +- ext/standard/password.c | 91 +++++++++++++--------------------- ext/standard/php_password.h | 6 +-- 3 files changed, 38 insertions(+), 63 deletions(-) diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c index 64025db625729..9e35a5e020b92 100644 --- a/ext/standard/basic_functions.c +++ b/ext/standard/basic_functions.c @@ -1867,7 +1867,7 @@ ZEND_BEGIN_ARG_INFO(arginfo_getlastmod, 0) ZEND_END_ARG_INFO() /* }}} */ /* {{{ password.c */ -ZEND_BEGIN_ARG_INFO_EX(arginfo_password_create, 0, 0, 1) +ZEND_BEGIN_ARG_INFO_EX(arginfo_password_hash, 0, 0, 1) ZEND_ARG_INFO(0, password) ZEND_ARG_INFO(0, algo) ZEND_ARG_INFO(0, options) @@ -2895,7 +2895,7 @@ const zend_function_entry basic_functions[] = { /* {{{ */ PHP_FE(base64_decode, arginfo_base64_decode) PHP_FE(base64_encode, arginfo_base64_encode) - PHP_FE(password_create, arginfo_password_create) + PHP_FE(password_hash, arginfo_password_hash) PHP_FE(password_verify, arginfo_password_verify) PHP_FE(password_make_salt, arginfo_password_make_salt) diff --git a/ext/standard/password.c b/ext/standard/password.c index f2c94fb473b5c..f049fbcbf1e8c 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -21,19 +21,24 @@ #include #include "php.h" -#include "ext/hash/php_hash.h" + +#include "fcntl.h" #include "php_password.h" #include "php_rand.h" #include "base64.h" #include "zend_interfaces.h" +#include "info.h" + +#if PHP_WIN32 +#include "win32/winutil.h" +#endif + + PHP_MINIT_FUNCTION(password) /* {{{ */ { REGISTER_STRING_CONSTANT("PASSWORD_DEFAULT", PHP_PASSWORD_DEFAULT, CONST_CS | CONST_PERSISTENT); REGISTER_STRING_CONSTANT("PASSWORD_BCRYPT", PHP_PASSWORD_BCRYPT, CONST_CS | CONST_PERSISTENT); - REGISTER_STRING_CONSTANT("PASSWORD_MD5", PHP_PASSWORD_MD5, CONST_CS | CONST_PERSISTENT); - REGISTER_STRING_CONSTANT("PASSWORD_SHA256", PHP_PASSWORD_SHA256, CONST_CS | CONST_PERSISTENT); - REGISTER_STRING_CONSTANT("PASSWORD_SHA512", PHP_PASSWORD_SHA512, CONST_CS | CONST_PERSISTENT); return SUCCESS; } /* }}} */ @@ -76,7 +81,6 @@ static int php_password_make_salt(int length, int raw, char *ret TSRMLS_DC) { int i, raw_length, buffer_valid = 0; char *buffer; - zend_function *func_ptr; if (raw) { raw_length = length; @@ -84,42 +88,37 @@ static int php_password_make_salt(int length, int raw, char *ret TSRMLS_DC) raw_length = length * 3 / 4 + 1; } buffer = (char *) emalloc(raw_length + 1); - - /* Temp Placeholder */ - if (PHP_PASSWORD_FUNCTION_EXISTS("mcrypt_create_iv", 16)) { - zval *ret, *size, *source; - ALLOC_INIT_ZVAL(size); - ZVAL_LONG(size, raw_length); - ALLOC_INIT_ZVAL(source) - ZVAL_LONG(source, 1); // MCRYPT_DEV_URANDOM - zend_call_method_with_2_params(NULL, NULL, NULL, "mcrypt_create_iv", &ret, size, source); - zval_ptr_dtor(&size); - zval_ptr_dtor(&source); - if (Z_TYPE_P(ret) == IS_STRING) { - memcpy(buffer, Z_STRVAL_P(ret), raw_length); + +#if PHP_WIN32 + { + BYTE *iv_b = (BYTE *) buffer; + if (php_win32_get_random_bytes(iv_b, (size_t) raw_length) == SUCCESS) { buffer_valid = 1; } - zval_ptr_dtor(&ret); } - if (!buffer_valid && PHP_PASSWORD_FUNCTION_EXISTS("openssl_random_pseudo_bytes", 27)) { - zval *ret, *size; - ALLOC_INIT_ZVAL(size); - ZVAL_LONG(size, raw_length); - zend_call_method_with_1_params(NULL, NULL, NULL, "openssl_random_pseudo_bytes", &ret, size); - zval_ptr_dtor(&size); - if (Z_TYPE_P(ret) == IS_STRING) { - memcpy(buffer, Z_STRVAL_P(ret), raw_length); +#else + { + int fd, n; + size_t read_bytes = 0; + fd = open("/dev/urandom", O_RDONLY); + if (fd >= 0) { + while (read_bytes < raw_length) { + n = read(fd, buffer + read_bytes, raw_length - read_bytes); + if (n < 0) { + break; + } + read_bytes += n; + } + close(fd); + } + if (read_bytes == raw_length) { buffer_valid = 1; } - zval_ptr_dtor(&ret); } - +#endif if (!buffer_valid) { - long number; for (i = 0; i < raw_length; i++) { - number = php_rand(TSRMLS_C); - RAND_RANGE(number, 0, 255, PHP_RAND_MAX); - buffer[i] = (char) number; + buffer[i] ^= (char) (255.0 * php_rand(TSRMLS_C) / RAND_MAX); } } /* /Temp Placeholder */ @@ -202,9 +201,9 @@ PHP_FUNCTION(password_make_salt) } -/* {{{ proto string password(string password, string algo = PASSWORD_DEFAULT, array options = array()) +/* {{{ proto string password_hash(string password, string algo = PASSWORD_DEFAULT, array options = array()) Hash a password */ -PHP_FUNCTION(password_create) +PHP_FUNCTION(password_hash) { char *algo = 0, *hash_format, *hash, *salt; int algo_len = 0, salt_len = 0, required_salt_len = 0, hash_format_len; @@ -213,7 +212,7 @@ PHP_FUNCTION(password_create) zend_function *func_ptr; if (!PHP_PASSWORD_FUNCTION_EXISTS("crypt", 5)) { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Crypt must be loaded for password_verify to function"); + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Crypt must be loaded for password_hash to function"); RETURN_FALSE; } @@ -246,26 +245,6 @@ PHP_FUNCTION(password_create) hash_format = emalloc(8); sprintf(hash_format, "$2y$%02d$", cost); hash_format_len = 7; - } else if (strcmp(algo, PHP_PASSWORD_MD5) == 0) { - required_salt_len = 12; - hash_format = emalloc(4); - memcpy(hash_format, "$1$", 3); - hash_format_len = 3; - } else if (strcmp(algo, PHP_PASSWORD_SHA256) == 0 || strcmp(algo, PHP_PASSWORD_SHA512) == 0) { - int rounds = PHP_PASSWORD_SHA_DEFAULT_ROUNDS; - if (options && zend_symtable_find(options, "rounds", 7, (void **) &option_buffer) == SUCCESS) { - convert_to_long_ex(option_buffer); - rounds = Z_LVAL_PP(option_buffer); - zval_ptr_dtor(option_buffer); - if (rounds < 1000 || rounds > 999999999) { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Invalid SHA rounds parameter specified: %d", rounds); - RETURN_FALSE; - } - } - required_salt_len = 16; - hash_format = emalloc(21); - sprintf(hash_format, "$%s$rounds=%d$", algo, rounds); - hash_format_len = strlen(hash_format); } else { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unknown password hashing algorithm: %s", algo); RETURN_FALSE; diff --git a/ext/standard/php_password.h b/ext/standard/php_password.h index 5967840d51dae..830d31ce64b2f 100644 --- a/ext/standard/php_password.h +++ b/ext/standard/php_password.h @@ -21,7 +21,7 @@ #ifndef PHP_PASSWORD_H #define PHP_PASSWORD_H -PHP_FUNCTION(password_create); +PHP_FUNCTION(password_hash); PHP_FUNCTION(password_verify); PHP_FUNCTION(password_make_salt); @@ -29,12 +29,8 @@ PHP_MINIT_FUNCTION(password); #define PHP_PASSWORD_DEFAULT "2y" #define PHP_PASSWORD_BCRYPT "2y" -#define PHP_PASSWORD_MD5 "1" -#define PHP_PASSWORD_SHA256 "5" -#define PHP_PASSWORD_SHA512 "6" #define PHP_PASSWORD_BCRYPT_DEFAULT_COST 12; -#define PHP_PASSWORD_SHA_DEFAULT_ROUNDS 5000; #endif From 232da90388de2a3ba4ad430d281469498e88aca2 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Tue, 26 Jun 2012 21:15:56 -0400 Subject: [PATCH 09/44] Implement php.ini setting password.bcrypt_cost --- ext/standard/basic_functions.c | 1 + ext/standard/password.c | 25 +++++++++++++++++++------ ext/standard/php_password.h | 4 +--- main/main.c | 2 ++ php.ini-development | 9 +++++++++ php.ini-production | 9 +++++++++ 6 files changed, 41 insertions(+), 9 deletions(-) diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c index 9e35a5e020b92..5dc86ab097857 100644 --- a/ext/standard/basic_functions.c +++ b/ext/standard/basic_functions.c @@ -3846,6 +3846,7 @@ PHP_MINFO_FUNCTION(basic) /* {{{ */ php_info_print_table_start(); BASIC_MINFO_SUBMODULE(dl) BASIC_MINFO_SUBMODULE(mail) + BASIC_MINFO_SUBMODULE(password) php_info_print_table_end(); BASIC_MINFO_SUBMODULE(assert) } diff --git a/ext/standard/password.c b/ext/standard/password.c index f049fbcbf1e8c..94aa4dc3e3ec3 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -43,6 +43,11 @@ PHP_MINIT_FUNCTION(password) /* {{{ */ } /* }}} */ +PHP_MINFO_FUNCTION(password) /* {{{ */ +{ + php_info_print_table_row(2, "Default Password BCrypt Cost", INI_STR("password.bcrypt_cost")); +} +/* }}} */ static int php_password_salt_is_alphabet(const char *str, const int len) { @@ -169,7 +174,11 @@ PHP_FUNCTION(password_verify) zval_ptr_dtor(&ret); RETURN_FALSE; } - + + /* We're using this method instead of == in order to provide + * resistence towards timing attacks. This is a constant time + * equality check that will always check every byte of both + * values. */ for (i = 0; i < Z_STRLEN_P(ret); i++) { status |= (Z_STRVAL_P(ret)[i] ^ Z_STRVAL_P(hash)[i]); } @@ -231,16 +240,20 @@ PHP_FUNCTION(password_hash) } if (strcmp(algo, PHP_PASSWORD_BCRYPT) == 0) { - int cost = PHP_PASSWORD_BCRYPT_DEFAULT_COST; + int cost = 0; + cost = (int) INI_INT("password.bcrypt_cost"); + if (options && zend_symtable_find(options, "cost", 5, (void **) &option_buffer) == SUCCESS) { convert_to_long_ex(option_buffer); cost = Z_LVAL_PP(option_buffer); zval_ptr_dtor(option_buffer); - if (cost < 4 || cost > 31) { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Invalid bcrypt cost parameter specified: %d", cost); - RETURN_FALSE; - } } + + if (cost < 4 || cost > 31) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Invalid bcrypt cost parameter specified: %d", cost); + RETURN_FALSE; + } + required_salt_len = 22; hash_format = emalloc(8); sprintf(hash_format, "$2y$%02d$", cost); diff --git a/ext/standard/php_password.h b/ext/standard/php_password.h index 830d31ce64b2f..81fe41f529f26 100644 --- a/ext/standard/php_password.h +++ b/ext/standard/php_password.h @@ -26,13 +26,11 @@ PHP_FUNCTION(password_verify); PHP_FUNCTION(password_make_salt); PHP_MINIT_FUNCTION(password); +PHP_MINFO_FUNCTION(password); #define PHP_PASSWORD_DEFAULT "2y" #define PHP_PASSWORD_BCRYPT "2y" -#define PHP_PASSWORD_BCRYPT_DEFAULT_COST 12; - - #endif diff --git a/main/main.c b/main/main.c index cc04b1317e991..e52c32c57dce8 100644 --- a/main/main.c +++ b/main/main.c @@ -540,6 +540,8 @@ PHP_INI_BEGIN() STD_PHP_INI_ENTRY("error_append_string", NULL, PHP_INI_ALL, OnUpdateString, error_append_string, php_core_globals, core_globals) STD_PHP_INI_ENTRY("error_prepend_string", NULL, PHP_INI_ALL, OnUpdateString, error_prepend_string, php_core_globals, core_globals) + PHP_INI_ENTRY("password.bcrypt_cost", "11", PHP_INI_ALL, NULL) + PHP_INI_ENTRY("SMTP", "localhost",PHP_INI_ALL, NULL) PHP_INI_ENTRY("smtp_port", "25", PHP_INI_ALL, NULL) STD_PHP_INI_BOOLEAN("mail.add_x_header", "0", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateBool, mail_x_header, php_core_globals, core_globals) diff --git a/php.ini-development b/php.ini-development index a5a7a4a81f818..5f1205e6a1de4 100644 --- a/php.ini-development +++ b/php.ini-development @@ -1359,6 +1359,15 @@ bcmath.scale = 0 ; http://php.net/browscap ;browscap = extra/browscap.ini +[password] +; The default cost of a bcrypt hash created using password_hash() +; Note that this is only the default, and can be overriden by the +; options argument to password_hash(). Additionally, it only affects +; newly created hashes. A higher value will make the generated +; hash more resistent to brute forcing, but will also use more CPU +; Default: 11 +; password.bcrypt_cost = 11 + [Session] ; Handler used to store/retrieve data. ; http://php.net/session.save-handler diff --git a/php.ini-production b/php.ini-production index 5d8f26e0fd3b7..927f305cde87b 100644 --- a/php.ini-production +++ b/php.ini-production @@ -1359,6 +1359,15 @@ bcmath.scale = 0 ; http://php.net/browscap ;browscap = extra/browscap.ini +[password] +; The default cost of a bcrypt hash created using password_hash() +; Note that this is only the default, and can be overriden by the +; options argument to password_hash(). Additionally, it only affects +; newly created hashes. A higher value will make the generated +; hash more resistent to brute forcing, but will also use more CPU +; Default: 11 +; password.bcrypt_cost = 11 + [Session] ; Handler used to store/retrieve data. ; http://php.net/session.save-handler From e505316aeba0fbb52cd21ff84af784a9d3e2b49a Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Tue, 26 Jun 2012 22:05:25 -0400 Subject: [PATCH 10/44] Add tests for password hashing --- .../password/password_bcrypt_errors.phpt | 28 +++++++++++++ .../tests/password/password_hash.phpt | 27 +++++++++++++ .../tests/password/password_hash_error.phpt | 38 ++++++++++++++++++ .../tests/password/password_make_salt.phpt | 40 +++++++++++++++++++ .../password/password_make_salt_error.phpt | 23 +++++++++++ .../tests/password/password_verify.phpt | 21 ++++++++++ .../tests/password/password_verify_error.phpt | 18 +++++++++ 7 files changed, 195 insertions(+) create mode 100644 ext/standard/tests/password/password_bcrypt_errors.phpt create mode 100644 ext/standard/tests/password/password_hash.phpt create mode 100644 ext/standard/tests/password/password_hash_error.phpt create mode 100644 ext/standard/tests/password/password_make_salt.phpt create mode 100644 ext/standard/tests/password/password_make_salt_error.phpt create mode 100644 ext/standard/tests/password/password_verify.phpt create mode 100644 ext/standard/tests/password/password_verify_error.phpt diff --git a/ext/standard/tests/password/password_bcrypt_errors.phpt b/ext/standard/tests/password/password_bcrypt_errors.phpt new file mode 100644 index 0000000000000..42238173501ab --- /dev/null +++ b/ext/standard/tests/password/password_bcrypt_errors.phpt @@ -0,0 +1,28 @@ +--TEST-- +Test error operation of password_hash() with bcrypt hashing +--FILE-- + 3))); + +var_dump(password_hash("foo", PASSWORD_BCRYPT, array("cost" => 32))); + +var_dump(password_hash("foo", PASSWORD_BCRYPT, array("salt" => "foo"))); + +var_dump(password_hash("foo", PASSWORD_BCRYPT, array("salt" => "123456789012345678901"))); + +?> +--EXPECTF-- +Warning: password_hash(): Invalid bcrypt cost parameter specified: 3 in %s on line %d +bool(false) + +Warning: password_hash(): Invalid bcrypt cost parameter specified: 32 in %s on line %d +bool(false) + +Warning: password_hash(): Provided salt is too short: 3 expecting 22 in %s on line %d +bool(false) + +Warning: password_hash(): Provided salt is too short: 21 expecting 22 in %s on line %d +bool(false) + diff --git a/ext/standard/tests/password/password_hash.phpt b/ext/standard/tests/password/password_hash.phpt new file mode 100644 index 0000000000000..ecefa10af3ac3 --- /dev/null +++ b/ext/standard/tests/password/password_hash.phpt @@ -0,0 +1,27 @@ +--TEST-- +Test normal operation of password_hash() +--FILE-- + 7, "salt" => "usesomesillystringforsalt"))); + +var_dump(password_hash("test", PASSWORD_BCRYPT, array("salt" => "123456789012345678901" . chr(0)))); + +echo "OK!"; +?> +--EXPECT-- +int(60) +bool(true) +string(60) "$2y$07$usesomesillystringfore2uDLvp1Ii2e./U9C8sBjqp8I90dH6hi" +string(60) "$2y$04$MTIzNDU2Nzg5MDEyMzQ1NekACxf2CF7ipfk/b9FllU9Fs8RcUm5UG" +OK! diff --git a/ext/standard/tests/password/password_hash_error.phpt b/ext/standard/tests/password/password_hash_error.phpt new file mode 100644 index 0000000000000..dfbb094b39da5 --- /dev/null +++ b/ext/standard/tests/password/password_hash_error.phpt @@ -0,0 +1,38 @@ +--TEST-- +Test error operation of password_hash() +--FILE-- + 13))); + +?> +--EXPECTF-- +Warning: password_hash() expects at least 1 parameter, 0 given in %s on line %d +bool(false) + +Warning: password_hash() expects parameter 2 to be string, array given in %s on line %d +bool(false) + +Warning: password_hash(): Unknown password hashing algorithm: bar in %s on line %d +bool(false) + +Warning: password_hash() expects parameter 3 to be array, string given in %s on line %d +bool(false) + +Warning: password_hash(): Password must be a string in %s on line %d +bool(false) + +Warning: password_hash(): Non-string salt parameter supplied in %s on line %d +bool(false) + diff --git a/ext/standard/tests/password/password_make_salt.phpt b/ext/standard/tests/password/password_make_salt.phpt new file mode 100644 index 0000000000000..63b56f8544657 --- /dev/null +++ b/ext/standard/tests/password/password_make_salt.phpt @@ -0,0 +1,40 @@ +--TEST-- +Test normal operation of password_make_salt() +--FILE-- + +--EXPECT-- +1 +2 +3 +4 +5 + +1 +2 +3 +4 +5 + +bool(true) +OK! diff --git a/ext/standard/tests/password/password_make_salt_error.phpt b/ext/standard/tests/password/password_make_salt_error.phpt new file mode 100644 index 0000000000000..7d79713e8ddd4 --- /dev/null +++ b/ext/standard/tests/password/password_make_salt_error.phpt @@ -0,0 +1,23 @@ +--TEST-- +Test error operation of password_make_salt() +--FILE-- + +--EXPECTF-- +Warning: password_make_salt() expects at least 1 parameter, 0 given in %s on line %d +bool(false) + +Warning: password_make_salt() expects parameter 1 to be long, string given in %s on line %d +bool(false) + +Warning: password_make_salt(): Length cannot be less than or equal zero: -1 in %s on line %d +bool(false) + diff --git a/ext/standard/tests/password/password_verify.phpt b/ext/standard/tests/password/password_verify.phpt new file mode 100644 index 0000000000000..e7ecc7edd3086 --- /dev/null +++ b/ext/standard/tests/password/password_verify.phpt @@ -0,0 +1,21 @@ +--TEST-- +Test normal operation of password_verify) +--FILE-- + +--EXPECT-- +bool(false) +bool(false) +bool(false) +bool(true) +OK! diff --git a/ext/standard/tests/password/password_verify_error.phpt b/ext/standard/tests/password/password_verify_error.phpt new file mode 100644 index 0000000000000..3e653fa04e475 --- /dev/null +++ b/ext/standard/tests/password/password_verify_error.phpt @@ -0,0 +1,18 @@ +--TEST-- +Test error operation of password_verify() +--FILE-- + +--EXPECTF-- +Warning: password_verify() expects exactly 2 parameters, 0 given in %s on line %d +bool(false) + +Warning: password_verify() expects exactly 2 parameters, 1 given in %s on line %d +bool(false) + From 2b9591f11f2573f8d9032477b7ad49c6cf92988c Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Tue, 26 Jun 2012 22:13:51 -0400 Subject: [PATCH 11/44] Update tests to check ini setting --- ext/standard/tests/password/password_hash.phpt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ext/standard/tests/password/password_hash.phpt b/ext/standard/tests/password/password_hash.phpt index ecefa10af3ac3..2fca8b71bc370 100644 --- a/ext/standard/tests/password/password_hash.phpt +++ b/ext/standard/tests/password/password_hash.phpt @@ -17,6 +17,11 @@ var_dump(password_hash("rasmuslerdorf", PASSWORD_BCRYPT, array("cost" => 7, "sal var_dump(password_hash("test", PASSWORD_BCRYPT, array("salt" => "123456789012345678901" . chr(0)))); +// test ini parameter to ensure that it updates +ini_set('password.bcrypt_cost', '5'); +var_dump(password_hash("test", PASSWORD_BCRYPT, array("salt" => "123456789012345678901" . chr(0)))); + + echo "OK!"; ?> --EXPECT-- @@ -24,4 +29,5 @@ int(60) bool(true) string(60) "$2y$07$usesomesillystringfore2uDLvp1Ii2e./U9C8sBjqp8I90dH6hi" string(60) "$2y$04$MTIzNDU2Nzg5MDEyMzQ1NekACxf2CF7ipfk/b9FllU9Fs8RcUm5UG" +string(60) "$2y$05$MTIzNDU2Nzg5MDEyMzQ1NeVt1jFvl6ZQVujUMmcYvue.Mr5oZVQa2" OK! From 5f44be03af7733c2618d980e77426572fb0148df Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Tue, 26 Jun 2012 23:09:08 -0400 Subject: [PATCH 12/44] Add tests and error checking for large salt requested values to prevent overflow on allocation --- ext/standard/password.c | 19 ++++++++++++++----- .../password/password_make_salt_error.phpt | 10 ++++++++++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index 94aa4dc3e3ec3..ab115afb6c7ec 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -82,14 +82,19 @@ static int php_password_salt_to64(const char *str, const int str_len, const int #define PHP_PASSWORD_FUNCTION_EXISTS(func, func_len) (zend_hash_find(EG(function_table), (func), (func_len) + 1, (void **) &func_ptr) == SUCCESS && func_ptr->type == ZEND_INTERNAL_FUNCTION && func_ptr->internal_function.handler != zif_display_disabled_function) -static int php_password_make_salt(int length, int raw, char *ret TSRMLS_DC) +static int php_password_make_salt(long length, int raw, char *ret TSRMLS_DC) { - int i, raw_length, buffer_valid = 0; + int buffer_valid = 0; + long i, raw_length; char *buffer; if (raw) { raw_length = length; } else { + if (length > (LONG_MAX / 3)) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Length is too large to safely generate"); + return FAILURE; + } raw_length = length * 3 / 4 + 1; } buffer = (char *) emalloc(raw_length + 1); @@ -192,15 +197,19 @@ PHP_FUNCTION(password_verify) PHP_FUNCTION(password_make_salt) { char *salt; - int length = 0; + long length = 0; zend_bool raw_output = 0; if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "l|b", &length, &raw_output) == FAILURE) { RETURN_FALSE; } if (length <= 0) { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Length cannot be less than or equal zero: %d", length); + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Length cannot be less than or equal zero: %ld", length); + RETURN_FALSE; + } else if (length > (LONG_MAX / 3)) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Length is too large to safely generate"); RETURN_FALSE; } + salt = emalloc(length + 1); if (php_password_make_salt(length, (int) raw_output, salt TSRMLS_CC) == FAILURE) { efree(salt); @@ -298,7 +307,7 @@ PHP_FUNCTION(password_hash) zval_ptr_dtor(option_buffer); } else { salt = emalloc(required_salt_len + 1); - if (php_password_make_salt(required_salt_len, 0, salt TSRMLS_CC) == FAILURE) { + if (php_password_make_salt((long) required_salt_len, 0, salt TSRMLS_CC) == FAILURE) { efree(hash_format); efree(salt); RETURN_FALSE; diff --git a/ext/standard/tests/password/password_make_salt_error.phpt b/ext/standard/tests/password/password_make_salt_error.phpt index 7d79713e8ddd4..8078582e3c828 100644 --- a/ext/standard/tests/password/password_make_salt_error.phpt +++ b/ext/standard/tests/password/password_make_salt_error.phpt @@ -10,6 +10,10 @@ var_dump(password_make_salt("foo")); var_dump(password_make_salt(-1)); +var_dump(password_make_salt(PHP_INT_MAX)); + +var_dump(password_make_salt(floor(PHP_INT_MAX / 2.9))); + ?> --EXPECTF-- Warning: password_make_salt() expects at least 1 parameter, 0 given in %s on line %d @@ -21,3 +25,9 @@ bool(false) Warning: password_make_salt(): Length cannot be less than or equal zero: -1 in %s on line %d bool(false) +Warning: password_make_salt(): Length is too large to safely generate in %s on line %d +bool(false) + +Warning: password_make_salt(): Length is too large to safely generate in %s on line %d +bool(false) + From 0dd2f16b148f4054d65645b9cf971fe08824d78d Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Wed, 27 Jun 2012 11:04:41 -0400 Subject: [PATCH 13/44] Fix formatting issues in password.c --- ext/standard/password.c | 139 +++++++++++++++++++++------------------- 1 file changed, 72 insertions(+), 67 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index ab115afb6c7ec..e0e260a0c190f 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -33,8 +33,6 @@ #include "win32/winutil.h" #endif - - PHP_MINIT_FUNCTION(password) /* {{{ */ { REGISTER_STRING_CONSTANT("PASSWORD_DEFAULT", PHP_PASSWORD_DEFAULT, CONST_CS | CONST_PERSISTENT); @@ -49,40 +47,42 @@ PHP_MINFO_FUNCTION(password) /* {{{ */ } /* }}} */ -static int php_password_salt_is_alphabet(const char *str, const int len) +static int php_password_salt_is_alphabet(const char *str, const int len) /* {{{ */ { - int i = 0; - - for (i = 0; i < len; i++) { - if (!((str[i] >= 'A' && str[i] <= 'Z') || (str[i] >= 'a' && str[i] <= 'z') || (str[i] >= '0' && str[i] <= '9') || str[i] == '.' || str[i] == '/')) { - return 0; - } - } - return 1; + int i = 0; + + for (i = 0; i < len; i++) { + if (!((str[i] >= 'A' && str[i] <= 'Z') || (str[i] >= 'a' && str[i] <= 'z') || (str[i] >= '0' && str[i] <= '9') || str[i] == '.' || str[i] == '/')) { + return 0; + } + } + return 1; } +/* }}} */ -static int php_password_salt_to64(const char *str, const int str_len, const int out_len, char *ret) +static int php_password_salt_to64(const char *str, const int str_len, const int out_len, char *ret) /* {{{ */ { - int pos = 0; + int pos = 0; unsigned char *buffer; - buffer = php_base64_encode((unsigned char*) str, str_len, NULL); - for (pos = 0; pos < out_len; pos++) { - if (buffer[pos] == '+') { - ret[pos] = '.'; + buffer = php_base64_encode((unsigned char*) str, str_len, NULL); + for (pos = 0; pos < out_len; pos++) { + if (buffer[pos] == '+') { + ret[pos] = '.'; } else if (buffer[pos] == '=') { efree(buffer); return FAILURE; - } else { + } else { ret[pos] = buffer[pos]; } - } + } efree(buffer); return SUCCESS; } +/* }}} */ #define PHP_PASSWORD_FUNCTION_EXISTS(func, func_len) (zend_hash_find(EG(function_table), (func), (func_len) + 1, (void **) &func_ptr) == SUCCESS && func_ptr->type == ZEND_INTERNAL_FUNCTION && func_ptr->internal_function.handler != zif_display_disabled_function) -static int php_password_make_salt(long length, int raw, char *ret TSRMLS_DC) +static int php_password_make_salt(long length, int raw, char *ret TSRMLS_DC) /* {{{ */ { int buffer_valid = 0; long i, raw_length; @@ -131,7 +131,6 @@ static int php_password_make_salt(long length, int raw, char *ret TSRMLS_DC) buffer[i] ^= (char) (255.0 * php_rand(TSRMLS_C) / RAND_MAX); } } - /* /Temp Placeholder */ if (raw) { memcpy(ret, buffer, length); @@ -151,8 +150,11 @@ static int php_password_make_salt(long length, int raw, char *ret TSRMLS_DC) efree(buffer); ret[length] = 0; return SUCCESS; -} +} +/* }}} */ +/* {{{ proto boolean password_make_salt(string password, string hash) +Verify a hash created using crypt() or password_hash() */ PHP_FUNCTION(password_verify) { zval *password, *hash, *ret; @@ -165,8 +167,8 @@ PHP_FUNCTION(password_verify) } if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "zz", &password, &hash) == FAILURE) { - RETURN_FALSE; - } + RETURN_FALSE; + } zend_call_method_with_2_params(NULL, NULL, NULL, "crypt", &ret, password, hash); @@ -193,15 +195,18 @@ PHP_FUNCTION(password_verify) RETURN_BOOL(status == 0); } +/* }}} */ +/* {{{ proto string password_make_salt(int length, boolean raw_output = false) +Make a new random salt */ PHP_FUNCTION(password_make_salt) { char *salt; long length = 0; zend_bool raw_output = 0; if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "l|b", &length, &raw_output) == FAILURE) { - RETURN_FALSE; - } + RETURN_FALSE; + } if (length <= 0) { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Length cannot be less than or equal zero: %ld", length); RETURN_FALSE; @@ -217,16 +222,16 @@ PHP_FUNCTION(password_make_salt) } RETURN_STRINGL(salt, length, 0); } - +/* }}} */ /* {{{ proto string password_hash(string password, string algo = PASSWORD_DEFAULT, array options = array()) Hash a password */ PHP_FUNCTION(password_hash) { - char *algo = 0, *hash_format, *hash, *salt; - int algo_len = 0, salt_len = 0, required_salt_len = 0, hash_format_len; - HashTable *options = 0; - zval **option_buffer, *ret, *password, *hash_zval; + char *algo = 0, *hash_format, *hash, *salt; + int algo_len = 0, salt_len = 0, required_salt_len = 0, hash_format_len; + HashTable *options = 0; + zval **option_buffer, *ret, *password, *hash_zval; zend_function *func_ptr; if (!PHP_PASSWORD_FUNCTION_EXISTS("crypt", 5)) { @@ -234,21 +239,21 @@ PHP_FUNCTION(password_hash) RETURN_FALSE; } - if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "z|sH", &password, &algo, &algo_len, &options) == FAILURE) { - RETURN_FALSE; - } + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "z|sH", &password, &algo, &algo_len, &options) == FAILURE) { + RETURN_FALSE; + } if (Z_TYPE_P(password) != IS_STRING) { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Password must be a string"); RETURN_FALSE; } - if (algo_len == 0) { + if (algo_len == 0) { algo = PHP_PASSWORD_DEFAULT; - algo_len = strlen(PHP_PASSWORD_DEFAULT); - } + algo_len = strlen(PHP_PASSWORD_DEFAULT); + } - if (strcmp(algo, PHP_PASSWORD_BCRYPT) == 0) { + if (strcmp(algo, PHP_PASSWORD_BCRYPT) == 0) { int cost = 0; cost = (int) INI_INT("password.bcrypt_cost"); @@ -260,60 +265,60 @@ PHP_FUNCTION(password_hash) if (cost < 4 || cost > 31) { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Invalid bcrypt cost parameter specified: %d", cost); - RETURN_FALSE; + RETURN_FALSE; } - required_salt_len = 22; + required_salt_len = 22; hash_format = emalloc(8); sprintf(hash_format, "$2y$%02d$", cost); hash_format_len = 7; - } else { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unknown password hashing algorithm: %s", algo); - RETURN_FALSE; - } + } else { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unknown password hashing algorithm: %s", algo); + RETURN_FALSE; + } - if (options && zend_symtable_find(options, "salt", 5, (void**) &option_buffer) == SUCCESS) { + if (options && zend_symtable_find(options, "salt", 5, (void**) &option_buffer) == SUCCESS) { char *buffer; int buffer_len; - if (Z_TYPE_PP(option_buffer) == IS_STRING) { - buffer = Z_STRVAL_PP(option_buffer); - buffer_len = Z_STRLEN_PP(option_buffer); - } else { - zval_ptr_dtor(option_buffer); + if (Z_TYPE_PP(option_buffer) == IS_STRING) { + buffer = Z_STRVAL_PP(option_buffer); + buffer_len = Z_STRLEN_PP(option_buffer); + } else { + zval_ptr_dtor(option_buffer); efree(hash_format); - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Non-string salt parameter supplied"); - RETURN_FALSE; - } - if (buffer_len < required_salt_len) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Non-string salt parameter supplied"); + RETURN_FALSE; + } + if (buffer_len < required_salt_len) { efree(hash_format); - zval_ptr_dtor(option_buffer); - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Provided salt is too short: %d expecting %d", buffer_len, required_salt_len); - RETURN_FALSE; - } else if (0 == php_password_salt_is_alphabet(buffer, buffer_len)) { + zval_ptr_dtor(option_buffer); + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Provided salt is too short: %d expecting %d", buffer_len, required_salt_len); + RETURN_FALSE; + } else if (0 == php_password_salt_is_alphabet(buffer, buffer_len)) { salt = emalloc(required_salt_len + 1); - if (php_password_salt_to64(buffer, buffer_len, required_salt_len, salt) == FAILURE) { + if (php_password_salt_to64(buffer, buffer_len, required_salt_len, salt) == FAILURE) { efree(hash_format); efree(salt); - zval_ptr_dtor(option_buffer); - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Provided salt is too short: %d", salt_len); + zval_ptr_dtor(option_buffer); + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Provided salt is too short: %d", salt_len); RETURN_FALSE; } - salt_len = required_salt_len; - } else { + salt_len = required_salt_len; + } else { salt = emalloc(required_salt_len + 1); memcpy(salt, buffer, required_salt_len); - salt_len = required_salt_len; + salt_len = required_salt_len; } zval_ptr_dtor(option_buffer); - } else { + } else { salt = emalloc(required_salt_len + 1); if (php_password_make_salt((long) required_salt_len, 0, salt TSRMLS_CC) == FAILURE) { efree(hash_format); efree(salt); RETURN_FALSE; } - salt_len = required_salt_len; - } + salt_len = required_salt_len; + } salt[salt_len] = 0; From 6bb3865a235d437d91df1940b0caad6995b69d4c Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Thu, 28 Jun 2012 14:44:04 -0400 Subject: [PATCH 14/44] Refactor crypt to use an external working function --- ext/standard/crypt.c | 188 +++++++++++++++++++++------------------ ext/standard/php_crypt.h | 1 + 2 files changed, 103 insertions(+), 86 deletions(-) diff --git a/ext/standard/crypt.c b/ext/standard/crypt.c index 9a1fcf1f6946c..a592a4b37c0a0 100644 --- a/ext/standard/crypt.c +++ b/ext/standard/crypt.c @@ -145,44 +145,9 @@ static void php_to64(char *s, long v, int n) /* {{{ */ } /* }}} */ -/* {{{ proto string crypt(string str [, string salt]) - Hash a string */ -PHP_FUNCTION(crypt) +PHPAPI int crypt_execute(const char *password, const int pass_len, const char *salt, int salt_len, char **result) { - char salt[PHP_MAX_SALT_LEN + 1]; - char *str, *salt_in = NULL; - int str_len, salt_in_len = 0; char *crypt_res; - salt[0] = salt[PHP_MAX_SALT_LEN] = '\0'; - - /* This will produce suitable results if people depend on DES-encryption - * available (passing always 2-character salt). At least for glibc6.1 */ - memset(&salt[1], '$', PHP_MAX_SALT_LEN - 1); - - if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s|s", &str, &str_len, &salt_in, &salt_in_len) == FAILURE) { - return; - } - - if (salt_in) { - memcpy(salt, salt_in, MIN(PHP_MAX_SALT_LEN, salt_in_len)); - } - - /* The automatic salt generation covers standard DES, md5-crypt and Blowfish (simple) */ - if (!*salt) { -#if PHP_MD5_CRYPT - strncpy(salt, "$1$", PHP_MAX_SALT_LEN); - php_to64(&salt[3], PHP_CRYPT_RAND, 4); - php_to64(&salt[7], PHP_CRYPT_RAND, 4); - strncpy(&salt[11], "$", PHP_MAX_SALT_LEN - 11); -#elif PHP_STD_DES_CRYPT - php_to64(&salt[0], PHP_CRYPT_RAND, 2); - salt[2] = '\0'; -#endif - salt_in_len = strlen(salt); - } else { - salt_in_len = MIN(PHP_MAX_SALT_LEN, salt_in_len); - } - /* Windows (win32/crypt) has a stripped down version of libxcrypt and a CryptoApi md5_crypt implementation */ #if PHP_USE_PHP_CRYPT_R @@ -190,55 +155,52 @@ PHP_FUNCTION(crypt) struct php_crypt_extended_data buffer; if (salt[0]=='$' && salt[1]=='1' && salt[2]=='$') { - char output[MD5_HASH_MAX_LEN]; - - RETURN_STRING(php_md5_crypt_r(str, salt, output), 1); + char output[MD5_HASH_MAX_LEN], *out; + + out = php_md5_crypt_r(password, salt, output); + if (out) { + *result = (char *) emalloc(MD5_HASH_MAX_LEN + 1); + memcpy(*result, out, MD5_HASH_MAX_LEN); + *result[MD5_HASH_MAX_LEN] = 0; + return SUCCESS; + } + return FAILURE; } else if (salt[0]=='$' && salt[1]=='6' && salt[2]=='$') { const char sha512_salt_prefix[] = "$6$"; const char sha512_rounds_prefix[] = "rounds="; char *output; int needed = (sizeof(sha512_salt_prefix) - 1 + sizeof(sha512_rounds_prefix) + 9 + 1 - + strlen(salt) + 1 + 43 + 1); + + PHP_MAX_SALT_LEN + 43 + 1); output = emalloc(needed); - salt[salt_in_len] = '\0'; - crypt_res = php_sha512_crypt_r(str, salt, output, needed); + crypt_res = php_sha512_crypt_r(password, salt, output, needed); if (!crypt_res) { - if (salt[0]=='*' && salt[1]=='0') { - RETVAL_STRING("*1", 1); - } else { - RETVAL_STRING("*0", 1); - } + memset(output, 0, needed); + efree(output); + return FAILURE; } else { - RETVAL_STRING(output, 1); + *result = output; + return SUCCESS; } - - memset(output, 0, PHP_MAX_SALT_LEN + 1); - efree(output); } else if (salt[0]=='$' && salt[1]=='5' && salt[2]=='$') { const char sha256_salt_prefix[] = "$5$"; const char sha256_rounds_prefix[] = "rounds="; char *output; int needed = (sizeof(sha256_salt_prefix) - 1 + sizeof(sha256_rounds_prefix) + 9 + 1 - + strlen(salt) + 1 + 43 + 1); + + PHP_MAX_SALT_LEN + 43 + 1); output = emalloc(needed); - salt[salt_in_len] = '\0'; - crypt_res = php_sha256_crypt_r(str, salt, output, needed); + crypt_res = php_sha256_crypt_r(password, salt, output, needed); if (!crypt_res) { - if (salt[0]=='*' && salt[1]=='0') { - RETVAL_STRING("*1", 1); - } else { - RETVAL_STRING("*0", 1); - } + memset(output, 0, needed); + efree(output); + return FAILURE; } else { - RETVAL_STRING(output, 1); + *result = output; + return SUCCESS; } - - memset(output, 0, PHP_MAX_SALT_LEN + 1); - efree(output); } else if ( salt[0] == '$' && salt[1] == '2' && @@ -251,31 +213,33 @@ PHP_FUNCTION(crypt) memset(output, 0, PHP_MAX_SALT_LEN + 1); - crypt_res = php_crypt_blowfish_rn(str, salt, output, sizeof(output)); + crypt_res = php_crypt_blowfish_rn(password, salt, output, sizeof(output)); if (!crypt_res) { - if (salt[0]=='*' && salt[1]=='0') { - RETVAL_STRING("*1", 1); - } else { - RETVAL_STRING("*0", 1); - } + memset(output, 0, PHP_MAX_SALT_LEN + 1); + return FAILURE; } else { - RETVAL_STRING(output, 1); + int result_len; + result_len = strlen(output); + *result = emalloc(result_len + 1); + memcpy(*result, output, result_len); + (*result)[result_len] = 0; + memset(output, 0, PHP_MAX_SALT_LEN + 1); + return SUCCESS; } - - memset(output, 0, PHP_MAX_SALT_LEN + 1); } else { memset(&buffer, 0, sizeof(buffer)); _crypt_extended_init_r(); - crypt_res = _crypt_extended_r(str, salt, &buffer); + crypt_res = _crypt_extended_r(password, salt, &buffer); if (!crypt_res) { - if (salt[0]=='*' && salt[1]=='0') { - RETURN_STRING("*1", 1); - } else { - RETURN_STRING("*0", 1); - } + return FAILURE; } else { - RETURN_STRING(crypt_res, 1); + int result_len; + result_len = strlen(crypt_res); + *result = emalloc(result_len + 1); + memcpy(*result, crypt_res, result_len); + (*result)[result_len] = 0; + return SUCCESS; } } } @@ -291,21 +255,73 @@ PHP_FUNCTION(crypt) # else # error Data struct used by crypt_r() is unknown. Please report. # endif - crypt_res = crypt_r(str, salt, &buffer); + crypt_res = crypt_r(password, salt, &buffer); if (!crypt_res) { - if (salt[0]=='*' && salt[1]=='0') { - RETURN_STRING("*1", 1); - } else { - RETURN_STRING("*0", 1); - } + return FAILURE; } else { - RETURN_STRING(crypt_res, 1); + int result_len; + result_len = strlen(crypt_res); + *result = emalloc(result_len + 1); + memcpy(*result, crypt_res, result_len); + (*result)[result_len] = '\0'; + return SUCCESS; } } # endif #endif } /* }}} */ + + +/* {{{ proto string crypt(string str [, string salt]) + Hash a string */ +PHP_FUNCTION(crypt) +{ + char salt[PHP_MAX_SALT_LEN + 1]; + char *str, *salt_in = NULL, *result = NULL; + int str_len, salt_in_len = 0; + salt[0] = salt[PHP_MAX_SALT_LEN] = '\0'; + + /* This will produce suitable results if people depend on DES-encryption + * available (passing always 2-character salt). At least for glibc6.1 */ + memset(&salt[1], '$', PHP_MAX_SALT_LEN - 1); + + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s|s", &str, &str_len, &salt_in, &salt_in_len) == FAILURE) { + return; + } + + if (salt_in) { + memcpy(salt, salt_in, MIN(PHP_MAX_SALT_LEN, salt_in_len)); + } + + /* The automatic salt generation covers standard DES, md5-crypt and Blowfish (simple) */ + if (!*salt) { +#if PHP_MD5_CRYPT + strncpy(salt, "$1$", PHP_MAX_SALT_LEN); + php_to64(&salt[3], PHP_CRYPT_RAND, 4); + php_to64(&salt[7], PHP_CRYPT_RAND, 4); + strncpy(&salt[11], "$", PHP_MAX_SALT_LEN - 11); +#elif PHP_STD_DES_CRYPT + php_to64(&salt[0], PHP_CRYPT_RAND, 2); + salt[2] = '\0'; +#endif + salt_in_len = strlen(salt); + } else { + salt_in_len = MIN(PHP_MAX_SALT_LEN, salt_in_len); + } + salt[salt_in_len] = '\0'; + + if (crypt_execute(str, str_len, salt, salt_in_len, &result) == FAILURE) { + if (salt[0] == '*' && salt[1] == '0') { + RETURN_STRING("*1", 1); + } else { + RETURN_STRING("*0", 1); + } + } + RETVAL_STRING(result, 1); + efree(result); +} +/* }}} */ #endif /* diff --git a/ext/standard/php_crypt.h b/ext/standard/php_crypt.h index 93b232896af37..1dffb0bc3ed29 100644 --- a/ext/standard/php_crypt.h +++ b/ext/standard/php_crypt.h @@ -23,6 +23,7 @@ #ifndef PHP_CRYPT_H #define PHP_CRYPT_H +PHPAPI int crypt_execute(const char *password, const int pass_len, const char *salt, int salt_len, char **result); PHP_FUNCTION(crypt); #if HAVE_CRYPT PHP_MINIT_FUNCTION(crypt); From da3d8bf514e61a486065b0bf335b4657f20e6b66 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Thu, 28 Jun 2012 15:29:40 -0400 Subject: [PATCH 15/44] Refactor password.c a bit, add different error checking --- ext/standard/password.c | 115 ++++++++---------- .../password/password_bcrypt_errors.phpt | 8 +- .../tests/password/password_hash_error.phpt | 18 +-- .../password/password_make_salt_error.phpt | 10 +- 4 files changed, 72 insertions(+), 79 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index e0e260a0c190f..dfe624dcefa86 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -21,10 +21,12 @@ #include #include "php.h" +#if HAVE_CRYPT #include "fcntl.h" #include "php_password.h" #include "php_rand.h" +#include "php_crypt.h" #include "base64.h" #include "zend_interfaces.h" #include "info.h" @@ -157,28 +159,19 @@ static int php_password_make_salt(long length, int raw, char *ret TSRMLS_DC) /* Verify a hash created using crypt() or password_hash() */ PHP_FUNCTION(password_verify) { - zval *password, *hash, *ret; int status = 0, i; - zend_function *func_ptr; - - if (!PHP_PASSWORD_FUNCTION_EXISTS("crypt", 5)) { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Crypt must be loaded for password_verify to function"); - RETURN_FALSE; - } - - if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "zz", &password, &hash) == FAILURE) { + int password_len, hash_len; + char *ret, *password, *hash; + + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "ss", &password, &password_len, &hash, &hash_len) == FAILURE) { RETURN_FALSE; } - - zend_call_method_with_2_params(NULL, NULL, NULL, "crypt", &ret, password, hash); - - if (Z_TYPE_P(ret) != IS_STRING) { - zval_ptr_dtor(&ret); + if (crypt_execute(password, password_len, hash, hash_len, &ret) == FAILURE) { RETURN_FALSE; } - if (Z_STRLEN_P(ret) != Z_STRLEN_P(hash)) { - zval_ptr_dtor(&ret); + if (strlen(ret) != hash_len) { + efree(ret); RETURN_FALSE; } @@ -186,11 +179,11 @@ PHP_FUNCTION(password_verify) * resistence towards timing attacks. This is a constant time * equality check that will always check every byte of both * values. */ - for (i = 0; i < Z_STRLEN_P(ret); i++) { - status |= (Z_STRVAL_P(ret)[i] ^ Z_STRVAL_P(hash)[i]); + for (i = 0; i < hash_len; i++) { + status |= (ret[i] ^ hash[i]); } - zval_ptr_dtor(&ret); + efree(ret); RETURN_BOOL(status == 0); @@ -205,14 +198,14 @@ PHP_FUNCTION(password_make_salt) long length = 0; zend_bool raw_output = 0; if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "l|b", &length, &raw_output) == FAILURE) { - RETURN_FALSE; + RETURN_NULL(); } if (length <= 0) { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Length cannot be less than or equal zero: %ld", length); - RETURN_FALSE; + RETURN_NULL(); } else if (length > (LONG_MAX / 3)) { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Length is too large to safely generate"); - RETURN_FALSE; + RETURN_NULL(); } salt = emalloc(length + 1); @@ -228,24 +221,13 @@ PHP_FUNCTION(password_make_salt) Hash a password */ PHP_FUNCTION(password_hash) { - char *algo = 0, *hash_format, *hash, *salt; - int algo_len = 0, salt_len = 0, required_salt_len = 0, hash_format_len; + char *algo = 0, *hash_format, *hash, *salt, *password, *result; + int algo_len = 0, salt_len = 0, required_salt_len = 0, hash_format_len, password_len; HashTable *options = 0; - zval **option_buffer, *ret, *password, *hash_zval; - zend_function *func_ptr; - - if (!PHP_PASSWORD_FUNCTION_EXISTS("crypt", 5)) { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Crypt must be loaded for password_hash to function"); - RETURN_FALSE; - } - - if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "z|sH", &password, &algo, &algo_len, &options) == FAILURE) { - RETURN_FALSE; - } + zval **option_buffer; - if (Z_TYPE_P(password) != IS_STRING) { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Password must be a string"); - RETURN_FALSE; + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s|sH", &password, &password_len, &algo, &algo_len, &options) == FAILURE) { + RETURN_NULL(); } if (algo_len == 0) { @@ -265,7 +247,7 @@ PHP_FUNCTION(password_hash) if (cost < 4 || cost > 31) { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Invalid bcrypt cost parameter specified: %d", cost); - RETURN_FALSE; + RETURN_NULL(); } required_salt_len = 22; @@ -274,26 +256,38 @@ PHP_FUNCTION(password_hash) hash_format_len = 7; } else { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unknown password hashing algorithm: %s", algo); - RETURN_FALSE; + RETURN_NULL(); } if (options && zend_symtable_find(options, "salt", 5, (void**) &option_buffer) == SUCCESS) { char *buffer; int buffer_len; - if (Z_TYPE_PP(option_buffer) == IS_STRING) { - buffer = Z_STRVAL_PP(option_buffer); - buffer_len = Z_STRLEN_PP(option_buffer); - } else { - zval_ptr_dtor(option_buffer); - efree(hash_format); - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Non-string salt parameter supplied"); - RETURN_FALSE; + switch (Z_TYPE_PP(option_buffer)) { + case IS_NULL: + case IS_STRING: + case IS_LONG: + case IS_DOUBLE: + case IS_BOOL: + case IS_OBJECT: + convert_to_string_ex(option_buffer); + if (Z_TYPE_PP(option_buffer) == IS_STRING) { + buffer = Z_STRVAL_PP(option_buffer); + buffer_len = Z_STRLEN_PP(option_buffer); + break; + } + case IS_RESOURCE: + case IS_ARRAY: + default: + zval_ptr_dtor(option_buffer); + efree(hash_format); + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Non-string salt parameter supplied"); + RETURN_NULL(); } if (buffer_len < required_salt_len) { efree(hash_format); zval_ptr_dtor(option_buffer); php_error_docref(NULL TSRMLS_CC, E_WARNING, "Provided salt is too short: %d expecting %d", buffer_len, required_salt_len); - RETURN_FALSE; + RETURN_NULL(); } else if (0 == php_password_salt_is_alphabet(buffer, buffer_len)) { salt = emalloc(required_salt_len + 1); if (php_password_salt_to64(buffer, buffer_len, required_salt_len, salt) == FAILURE) { @@ -301,7 +295,7 @@ PHP_FUNCTION(password_hash) efree(salt); zval_ptr_dtor(option_buffer); php_error_docref(NULL TSRMLS_CC, E_WARNING, "Provided salt is too short: %d", salt_len); - RETURN_FALSE; + RETURN_NULL(); } salt_len = required_salt_len; } else { @@ -326,28 +320,27 @@ PHP_FUNCTION(password_hash) sprintf(hash, "%s%s", hash_format, salt); hash[hash_format_len + salt_len] = 0; - ALLOC_INIT_ZVAL(hash_zval); - ZVAL_STRINGL(hash_zval, hash, hash_format_len + salt_len, 0); - efree(hash_format); efree(salt); - zend_call_method_with_2_params(NULL, NULL, NULL, "crypt", &ret, password, hash_zval); + if (crypt_execute(password, password_len, hash, hash_format_len + salt_len, &result) == FAILURE) { + efree(hash); + RETURN_FALSE; + } - zval_ptr_dtor(&hash_zval); + efree(hash); - if (Z_TYPE_P(ret) != IS_STRING) { - zval_ptr_dtor(&ret); - RETURN_FALSE; - } else if(Z_STRLEN_P(ret) < 13) { - zval_ptr_dtor(&ret); + if (strlen(result) < 13) { + efree(result); RETURN_FALSE; } - RETURN_ZVAL(ret, 0, 1); + RETVAL_STRING(result, 1); + efree(result); } /* }}} */ +#endif /* HAVE_CRYPT */ /* * Local variables: * tab-width: 4 diff --git a/ext/standard/tests/password/password_bcrypt_errors.phpt b/ext/standard/tests/password/password_bcrypt_errors.phpt index 42238173501ab..f36d11f694185 100644 --- a/ext/standard/tests/password/password_bcrypt_errors.phpt +++ b/ext/standard/tests/password/password_bcrypt_errors.phpt @@ -15,14 +15,14 @@ var_dump(password_hash("foo", PASSWORD_BCRYPT, array("salt" => "1234567890123456 ?> --EXPECTF-- Warning: password_hash(): Invalid bcrypt cost parameter specified: 3 in %s on line %d -bool(false) +NULL Warning: password_hash(): Invalid bcrypt cost parameter specified: 32 in %s on line %d -bool(false) +NULL Warning: password_hash(): Provided salt is too short: 3 expecting 22 in %s on line %d -bool(false) +NULL Warning: password_hash(): Provided salt is too short: 21 expecting 22 in %s on line %d -bool(false) +NULL diff --git a/ext/standard/tests/password/password_hash_error.phpt b/ext/standard/tests/password/password_hash_error.phpt index dfbb094b39da5..b82e23edc036e 100644 --- a/ext/standard/tests/password/password_hash_error.phpt +++ b/ext/standard/tests/password/password_hash_error.phpt @@ -12,27 +12,27 @@ var_dump(password_hash("foo", "bar", new StdClass)); var_dump(password_hash("foo", "bar", "baz")); -var_dump(password_hash(123)); +var_dump(password_hash(array(), PASSWORD_BCRYPT)); -var_dump(password_hash("123", PASSWORD_BCRYPT, array("salt" => 13))); +var_dump(password_hash("123", PASSWORD_BCRYPT, array("salt" => array()))); ?> --EXPECTF-- Warning: password_hash() expects at least 1 parameter, 0 given in %s on line %d -bool(false) +NULL Warning: password_hash() expects parameter 2 to be string, array given in %s on line %d -bool(false) +NULL Warning: password_hash(): Unknown password hashing algorithm: bar in %s on line %d -bool(false) +NULL Warning: password_hash() expects parameter 3 to be array, string given in %s on line %d -bool(false) +NULL -Warning: password_hash(): Password must be a string in %s on line %d -bool(false) +Warning: password_hash() expects parameter 1 to be string, array given in %s on line %d +NULL Warning: password_hash(): Non-string salt parameter supplied in %s on line %d -bool(false) +NULL diff --git a/ext/standard/tests/password/password_make_salt_error.phpt b/ext/standard/tests/password/password_make_salt_error.phpt index 8078582e3c828..4a1d5e29c9837 100644 --- a/ext/standard/tests/password/password_make_salt_error.phpt +++ b/ext/standard/tests/password/password_make_salt_error.phpt @@ -17,17 +17,17 @@ var_dump(password_make_salt(floor(PHP_INT_MAX / 2.9))); ?> --EXPECTF-- Warning: password_make_salt() expects at least 1 parameter, 0 given in %s on line %d -bool(false) +NULL Warning: password_make_salt() expects parameter 1 to be long, string given in %s on line %d -bool(false) +NULL Warning: password_make_salt(): Length cannot be less than or equal zero: -1 in %s on line %d -bool(false) +NULL Warning: password_make_salt(): Length is too large to safely generate in %s on line %d -bool(false) +NULL Warning: password_make_salt(): Length is too large to safely generate in %s on line %d -bool(false) +NULL From 9c1445c6bcee99dbe1eeb9eb8eb6cd626ca72a9c Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Fri, 29 Jun 2012 11:32:25 -0400 Subject: [PATCH 16/44] More refactoring of crypt into php_crypt, and fixing memory allocation --- ext/standard/crypt.c | 59 +++++++++++++--------------------------- ext/standard/password.c | 7 ++--- ext/standard/php_crypt.h | 2 +- 3 files changed, 23 insertions(+), 45 deletions(-) diff --git a/ext/standard/crypt.c b/ext/standard/crypt.c index 25f5ec0107a22..3b443fc4d5a31 100644 --- a/ext/standard/crypt.c +++ b/ext/standard/crypt.c @@ -145,7 +145,7 @@ static void php_to64(char *s, long v, int n) /* {{{ */ } /* }}} */ -PHPAPI int crypt_execute(const char *password, const int pass_len, const char *salt, int salt_len, char **result) +PHPAPI int php_crypt(const char *password, const int pass_len, const char *salt, int salt_len, char **result) { char *crypt_res; /* Windows (win32/crypt) has a stripped down version of libxcrypt and @@ -159,46 +159,38 @@ PHPAPI int crypt_execute(const char *password, const int pass_len, const char *s out = php_md5_crypt_r(password, salt, output); if (out) { - *result = (char *) emalloc(MD5_HASH_MAX_LEN + 1); - memcpy(*result, out, MD5_HASH_MAX_LEN); - *result[MD5_HASH_MAX_LEN] = 0; + *result = estrdup(out); return SUCCESS; } return FAILURE; } else if (salt[0]=='$' && salt[1]=='6' && salt[2]=='$') { - const char sha512_salt_prefix[] = "$6$"; - const char sha512_rounds_prefix[] = "rounds="; char *output; - int needed = (sizeof(sha512_salt_prefix) - 1 - + sizeof(sha512_rounds_prefix) + 9 + 1 - + salt_in_len + 1 + 86 + 1); - output = emalloc(needed); + output = emalloc(PHP_MAX_SALT_LEN); - crypt_res = php_sha512_crypt_r(password, salt, output, needed); + crypt_res = php_sha512_crypt_r(password, salt, output, PHP_MAX_SALT_LEN); if (!crypt_res) { - memset(output, 0, needed); + memset(output, 0, PHP_MAX_SALT_LEN); efree(output); return FAILURE; } else { - *result = output; + *result = estrdup(output); + memset(output, 0, PHP_MAX_SALT_LEN); + efree(output); return SUCCESS; } } else if (salt[0]=='$' && salt[1]=='5' && salt[2]=='$') { - const char sha256_salt_prefix[] = "$5$"; - const char sha256_rounds_prefix[] = "rounds="; char *output; - int needed = (sizeof(sha256_salt_prefix) - 1 - + sizeof(sha256_rounds_prefix) + 9 + 1 - + salt_in_len + 1 + 43 + 1); - output = emalloc(needed); + output = emalloc(PHP_MAX_SALT_LEN); - crypt_res = php_sha256_crypt_r(password, salt, output, needed); + crypt_res = php_sha256_crypt_r(password, salt, output, PHP_MAX_SALT_LEN); if (!crypt_res) { - memset(output, 0, needed); + memset(output, 0, PHP_MAX_SALT_LEN); efree(output); return FAILURE; } else { - *result = output; + *result = estrdup(output); + memset(output, 0, PHP_MAX_SALT_LEN); + efree(output); return SUCCESS; } } else if ( @@ -218,11 +210,7 @@ PHPAPI int crypt_execute(const char *password, const int pass_len, const char *s memset(output, 0, PHP_MAX_SALT_LEN + 1); return FAILURE; } else { - int result_len; - result_len = strlen(output); - *result = emalloc(result_len + 1); - memcpy(*result, output, result_len); - (*result)[result_len] = 0; + *result = estrdup(output); memset(output, 0, PHP_MAX_SALT_LEN + 1); return SUCCESS; } @@ -234,11 +222,7 @@ PHPAPI int crypt_execute(const char *password, const int pass_len, const char *s if (!crypt_res) { return FAILURE; } else { - int result_len; - result_len = strlen(crypt_res); - *result = emalloc(result_len + 1); - memcpy(*result, crypt_res, result_len); - (*result)[result_len] = 0; + *result = estrdup(crypt_res); return SUCCESS; } } @@ -259,11 +243,7 @@ PHPAPI int crypt_execute(const char *password, const int pass_len, const char *s if (!crypt_res) { return FAILURE; } else { - int result_len; - result_len = strlen(crypt_res); - *result = emalloc(result_len + 1); - memcpy(*result, crypt_res, result_len); - (*result)[result_len] = '\0'; + *result = estrdup(crypt_res); return SUCCESS; } } @@ -311,15 +291,14 @@ PHP_FUNCTION(crypt) } salt[salt_in_len] = '\0'; - if (crypt_execute(str, str_len, salt, salt_in_len, &result) == FAILURE) { + if (php_crypt(str, str_len, salt, salt_in_len, &result) == FAILURE) { if (salt[0] == '*' && salt[1] == '0') { RETURN_STRING("*1", 1); } else { RETURN_STRING("*0", 1); } } - RETVAL_STRING(result, 1); - efree(result); + RETURN_STRING(result, 0); } /* }}} */ #endif diff --git a/ext/standard/password.c b/ext/standard/password.c index dfe624dcefa86..982ae7d5ac307 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -166,7 +166,7 @@ PHP_FUNCTION(password_verify) if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "ss", &password, &password_len, &hash, &hash_len) == FAILURE) { RETURN_FALSE; } - if (crypt_execute(password, password_len, hash, hash_len, &ret) == FAILURE) { + if (php_crypt(password, password_len, hash, hash_len, &ret) == FAILURE) { RETURN_FALSE; } @@ -323,7 +323,7 @@ PHP_FUNCTION(password_hash) efree(hash_format); efree(salt); - if (crypt_execute(password, password_len, hash, hash_format_len + salt_len, &result) == FAILURE) { + if (php_crypt(password, password_len, hash, hash_format_len + salt_len, &result) == FAILURE) { efree(hash); RETURN_FALSE; } @@ -335,8 +335,7 @@ PHP_FUNCTION(password_hash) RETURN_FALSE; } - RETVAL_STRING(result, 1); - efree(result); + RETURN_STRING(result, 0); } /* }}} */ diff --git a/ext/standard/php_crypt.h b/ext/standard/php_crypt.h index 1dffb0bc3ed29..7410a8c3280b9 100644 --- a/ext/standard/php_crypt.h +++ b/ext/standard/php_crypt.h @@ -23,7 +23,7 @@ #ifndef PHP_CRYPT_H #define PHP_CRYPT_H -PHPAPI int crypt_execute(const char *password, const int pass_len, const char *salt, int salt_len, char **result); +PHPAPI int php_crypt(const char *password, const int pass_len, const char *salt, int salt_len, char **result); PHP_FUNCTION(crypt); #if HAVE_CRYPT PHP_MINIT_FUNCTION(crypt); From f53112fdcf746ef73660059e72f8798d0108acac Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Fri, 29 Jun 2012 11:37:39 -0400 Subject: [PATCH 17/44] Update password.c to use safe_emalloc in sensitive places --- ext/standard/password.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index 982ae7d5ac307..558cf24c19516 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -99,7 +99,7 @@ static int php_password_make_salt(long length, int raw, char *ret TSRMLS_DC) /* } raw_length = length * 3 / 4 + 1; } - buffer = (char *) emalloc(raw_length + 1); + buffer = (char *) safe_emalloc(raw_length, 1, 1); #if PHP_WIN32 { @@ -138,7 +138,7 @@ static int php_password_make_salt(long length, int raw, char *ret TSRMLS_DC) /* memcpy(ret, buffer, length); } else { char *result; - result = emalloc(length + 1); + result = safe_emalloc(length, 1, 1); if (php_password_salt_to64(buffer, raw_length, length, result) == FAILURE) { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Generated salt too short"); efree(buffer); @@ -208,7 +208,7 @@ PHP_FUNCTION(password_make_salt) RETURN_NULL(); } - salt = emalloc(length + 1); + salt = safe_emalloc(length, 1, 1); if (php_password_make_salt(length, (int) raw_output, salt TSRMLS_CC) == FAILURE) { efree(salt); RETURN_FALSE; @@ -316,7 +316,7 @@ PHP_FUNCTION(password_hash) salt[salt_len] = 0; - hash = emalloc(salt_len + hash_format_len + 1); + hash = safe_emalloc(salt_len + hash_format_len, 1, 1); sprintf(hash, "%s%s", hash_format, salt); hash[hash_format_len + salt_len] = 0; From 6cc3c65fbf06da075934c89e470fa776d4d968fa Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Tue, 3 Jul 2012 07:33:55 -0400 Subject: [PATCH 18/44] Remove php.ini setting for default bcrypt cost --- ext/standard/password.c | 9 +-------- ext/standard/php_password.h | 3 ++- ext/standard/tests/password/password_hash.phpt | 12 ++---------- php.ini-development | 9 --------- php.ini-production | 9 --------- 5 files changed, 5 insertions(+), 37 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index 558cf24c19516..9c03152426210 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -43,12 +43,6 @@ PHP_MINIT_FUNCTION(password) /* {{{ */ } /* }}} */ -PHP_MINFO_FUNCTION(password) /* {{{ */ -{ - php_info_print_table_row(2, "Default Password BCrypt Cost", INI_STR("password.bcrypt_cost")); -} -/* }}} */ - static int php_password_salt_is_alphabet(const char *str, const int len) /* {{{ */ { int i = 0; @@ -236,8 +230,7 @@ PHP_FUNCTION(password_hash) } if (strcmp(algo, PHP_PASSWORD_BCRYPT) == 0) { - int cost = 0; - cost = (int) INI_INT("password.bcrypt_cost"); + int cost = PHP_PASSWORD_BCRYPT_COST; if (options && zend_symtable_find(options, "cost", 5, (void **) &option_buffer) == SUCCESS) { convert_to_long_ex(option_buffer); diff --git a/ext/standard/php_password.h b/ext/standard/php_password.h index 81fe41f529f26..338665ea2f94e 100644 --- a/ext/standard/php_password.h +++ b/ext/standard/php_password.h @@ -26,11 +26,12 @@ PHP_FUNCTION(password_verify); PHP_FUNCTION(password_make_salt); PHP_MINIT_FUNCTION(password); -PHP_MINFO_FUNCTION(password); #define PHP_PASSWORD_DEFAULT "2y" #define PHP_PASSWORD_BCRYPT "2y" +#define PHP_PASSWORD_BCRYPT_COST 10 + #endif diff --git a/ext/standard/tests/password/password_hash.phpt b/ext/standard/tests/password/password_hash.phpt index 2fca8b71bc370..3b6fc0932ca7d 100644 --- a/ext/standard/tests/password/password_hash.phpt +++ b/ext/standard/tests/password/password_hash.phpt @@ -4,9 +4,6 @@ Test normal operation of password_hash() 7, "sal var_dump(password_hash("test", PASSWORD_BCRYPT, array("salt" => "123456789012345678901" . chr(0)))); -// test ini parameter to ensure that it updates -ini_set('password.bcrypt_cost', '5'); -var_dump(password_hash("test", PASSWORD_BCRYPT, array("salt" => "123456789012345678901" . chr(0)))); - - echo "OK!"; ?> --EXPECT-- int(60) bool(true) string(60) "$2y$07$usesomesillystringfore2uDLvp1Ii2e./U9C8sBjqp8I90dH6hi" -string(60) "$2y$04$MTIzNDU2Nzg5MDEyMzQ1NekACxf2CF7ipfk/b9FllU9Fs8RcUm5UG" -string(60) "$2y$05$MTIzNDU2Nzg5MDEyMzQ1NeVt1jFvl6ZQVujUMmcYvue.Mr5oZVQa2" +string(60) "$2y$10$MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR.oP7XOR9HD/vjUuOj100y" OK! + diff --git a/php.ini-development b/php.ini-development index 5f1205e6a1de4..a5a7a4a81f818 100644 --- a/php.ini-development +++ b/php.ini-development @@ -1359,15 +1359,6 @@ bcmath.scale = 0 ; http://php.net/browscap ;browscap = extra/browscap.ini -[password] -; The default cost of a bcrypt hash created using password_hash() -; Note that this is only the default, and can be overriden by the -; options argument to password_hash(). Additionally, it only affects -; newly created hashes. A higher value will make the generated -; hash more resistent to brute forcing, but will also use more CPU -; Default: 11 -; password.bcrypt_cost = 11 - [Session] ; Handler used to store/retrieve data. ; http://php.net/session.save-handler diff --git a/php.ini-production b/php.ini-production index 927f305cde87b..5d8f26e0fd3b7 100644 --- a/php.ini-production +++ b/php.ini-production @@ -1359,15 +1359,6 @@ bcmath.scale = 0 ; http://php.net/browscap ;browscap = extra/browscap.ini -[password] -; The default cost of a bcrypt hash created using password_hash() -; Note that this is only the default, and can be overriden by the -; options argument to password_hash(). Additionally, it only affects -; newly created hashes. A higher value will make the generated -; hash more resistent to brute forcing, but will also use more CPU -; Default: 11 -; password.bcrypt_cost = 11 - [Session] ; Handler used to store/retrieve data. ; http://php.net/session.save-handler From 6943f2ab7f729d26281f9358dba27890d07dd24d Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Tue, 3 Jul 2012 08:24:31 -0400 Subject: [PATCH 19/44] Some more refactoring, make algo no longer optional --- ext/standard/basic_functions.c | 1 - ext/standard/password.c | 59 +++++++++---------- ext/standard/php_password.h | 4 +- .../tests/password/password_hash.phpt | 4 +- .../tests/password/password_hash_error.phpt | 15 +++-- 5 files changed, 43 insertions(+), 40 deletions(-) diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c index 5dc86ab097857..9e35a5e020b92 100644 --- a/ext/standard/basic_functions.c +++ b/ext/standard/basic_functions.c @@ -3846,7 +3846,6 @@ PHP_MINFO_FUNCTION(basic) /* {{{ */ php_info_print_table_start(); BASIC_MINFO_SUBMODULE(dl) BASIC_MINFO_SUBMODULE(mail) - BASIC_MINFO_SUBMODULE(password) php_info_print_table_end(); BASIC_MINFO_SUBMODULE(assert) } diff --git a/ext/standard/password.c b/ext/standard/password.c index 9c03152426210..6de812057f231 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -37,8 +37,8 @@ PHP_MINIT_FUNCTION(password) /* {{{ */ { - REGISTER_STRING_CONSTANT("PASSWORD_DEFAULT", PHP_PASSWORD_DEFAULT, CONST_CS | CONST_PERSISTENT); - REGISTER_STRING_CONSTANT("PASSWORD_BCRYPT", PHP_PASSWORD_BCRYPT, CONST_CS | CONST_PERSISTENT); + REGISTER_LONG_CONSTANT("PASSWORD_DEFAULT", PHP_PASSWORD_DEFAULT, CONST_CS | CONST_PERSISTENT); + REGISTER_LONG_CONSTANT("PASSWORD_BCRYPT", PHP_PASSWORD_BCRYPT, CONST_CS | CONST_PERSISTENT); return SUCCESS; } /* }}} */ @@ -211,45 +211,44 @@ PHP_FUNCTION(password_make_salt) } /* }}} */ -/* {{{ proto string password_hash(string password, string algo = PASSWORD_DEFAULT, array options = array()) +/* {{{ proto string password_hash(string password, string algo, array options = array()) Hash a password */ PHP_FUNCTION(password_hash) { - char *algo = 0, *hash_format, *hash, *salt, *password, *result; - int algo_len = 0, salt_len = 0, required_salt_len = 0, hash_format_len, password_len; + char *hash_format, *hash, *salt, *password, *result; + int algo = 0, salt_len = 0, required_salt_len = 0, hash_format_len, password_len; HashTable *options = 0; zval **option_buffer; - if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s|sH", &password, &password_len, &algo, &algo_len, &options) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "sl|H", &password, &password_len, &algo, &options) == FAILURE) { RETURN_NULL(); } - if (algo_len == 0) { - algo = PHP_PASSWORD_DEFAULT; - algo_len = strlen(PHP_PASSWORD_DEFAULT); - } - - if (strcmp(algo, PHP_PASSWORD_BCRYPT) == 0) { - int cost = PHP_PASSWORD_BCRYPT_COST; - - if (options && zend_symtable_find(options, "cost", 5, (void **) &option_buffer) == SUCCESS) { - convert_to_long_ex(option_buffer); - cost = Z_LVAL_PP(option_buffer); - zval_ptr_dtor(option_buffer); + switch (algo) { + case PHP_PASSWORD_BCRYPT: + { + int cost = PHP_PASSWORD_BCRYPT_COST; + + if (options && zend_symtable_find(options, "cost", 5, (void **) &option_buffer) == SUCCESS) { + convert_to_long_ex(option_buffer); + cost = Z_LVAL_PP(option_buffer); + zval_ptr_dtor(option_buffer); + } + + if (cost < 4 || cost > 31) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Invalid bcrypt cost parameter specified: %d", cost); + RETURN_NULL(); + } + + required_salt_len = 22; + hash_format = emalloc(8); + sprintf(hash_format, "$2y$%02d$", cost); + hash_format_len = 7; } - - if (cost < 4 || cost > 31) { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Invalid bcrypt cost parameter specified: %d", cost); + break; + default: + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unknown password hashing algorithm: %d", algo); RETURN_NULL(); - } - - required_salt_len = 22; - hash_format = emalloc(8); - sprintf(hash_format, "$2y$%02d$", cost); - hash_format_len = 7; - } else { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unknown password hashing algorithm: %s", algo); - RETURN_NULL(); } if (options && zend_symtable_find(options, "salt", 5, (void**) &option_buffer) == SUCCESS) { diff --git a/ext/standard/php_password.h b/ext/standard/php_password.h index 338665ea2f94e..57c6b8878580b 100644 --- a/ext/standard/php_password.h +++ b/ext/standard/php_password.h @@ -27,8 +27,8 @@ PHP_FUNCTION(password_make_salt); PHP_MINIT_FUNCTION(password); -#define PHP_PASSWORD_DEFAULT "2y" -#define PHP_PASSWORD_BCRYPT "2y" +#define PHP_PASSWORD_DEFAULT 1 +#define PHP_PASSWORD_BCRYPT 1 #define PHP_PASSWORD_BCRYPT_COST 10 diff --git a/ext/standard/tests/password/password_hash.phpt b/ext/standard/tests/password/password_hash.phpt index 3b6fc0932ca7d..ff48b29b169a9 100644 --- a/ext/standard/tests/password/password_hash.phpt +++ b/ext/standard/tests/password/password_hash.phpt @@ -4,9 +4,9 @@ Test normal operation of password_hash() array()))); ?> --EXPECTF-- -Warning: password_hash() expects at least 1 parameter, 0 given in %s on line %d +Warning: password_hash() expects at least 2 parameters, 0 given in %s on line %d +NULL + +Warning: password_hash() expects at least 2 parameters, 1 given in %s on line %d NULL -Warning: password_hash() expects parameter 2 to be string, array given in %s on line %d +Warning: password_hash() expects parameter 2 to be long, array given in %s on line %d NULL -Warning: password_hash(): Unknown password hashing algorithm: bar in %s on line %d +Warning: password_hash(): Unknown password hashing algorithm: 19 in %s on line %d NULL Warning: password_hash() expects parameter 3 to be array, string given in %s on line %d From 886527de56ecdd412a80a2901b8a0e3b622f037c Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Tue, 3 Jul 2012 08:26:50 -0400 Subject: [PATCH 20/44] Update signature info for changing algo to an ordinal --- ext/standard/password.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index 6de812057f231..eb4abd2722fdb 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -211,7 +211,7 @@ PHP_FUNCTION(password_make_salt) } /* }}} */ -/* {{{ proto string password_hash(string password, string algo, array options = array()) +/* {{{ proto string password_hash(string password, int algo, array options = array()) Hash a password */ PHP_FUNCTION(password_hash) { From 5160dc11cd9d0e97eb59138f4639e5af0584f370 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Thu, 5 Jul 2012 16:22:49 -0400 Subject: [PATCH 21/44] Implement password_needs_rehash() function --- ext/standard/basic_functions.c | 6 ++++ ext/standard/password.c | 50 ++++++++++++++++++++++++++++++++++ ext/standard/php_password.h | 1 + 3 files changed, 57 insertions(+) diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c index 9e35a5e020b92..bf6f9b0b583c5 100644 --- a/ext/standard/basic_functions.c +++ b/ext/standard/basic_functions.c @@ -1872,6 +1872,11 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_password_hash, 0, 0, 1) ZEND_ARG_INFO(0, algo) ZEND_ARG_INFO(0, options) ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_INFO_EX(arginfo_password_needs_rehash, 0, 0, 1) + ZEND_ARG_INFO(0, hash) + ZEND_ARG_INFO(0, algo) + ZEND_ARG_INFO(0, options) +ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_INFO_EX(arginfo_password_verify, 0, 0, 2) ZEND_ARG_INFO(0, password) ZEND_ARG_INFO(0, hash) @@ -2896,6 +2901,7 @@ const zend_function_entry basic_functions[] = { /* {{{ */ PHP_FE(base64_encode, arginfo_base64_encode) PHP_FE(password_hash, arginfo_password_hash) + PHP_FE(password_needs_rehash, arginfo_password_needs_rehash) PHP_FE(password_verify, arginfo_password_verify) PHP_FE(password_make_salt, arginfo_password_make_salt) diff --git a/ext/standard/password.c b/ext/standard/password.c index eb4abd2722fdb..9bfb02358436c 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -43,6 +43,18 @@ PHP_MINIT_FUNCTION(password) /* {{{ */ } /* }}} */ +static long php_password_determine_algo(const char *hash, const int len) +{ + if (len < 3) { + return 0; + } + if (hash[0] == '$' && hash[1] == '2' && hash[2] == 'y' && len == 60) { + return PHP_PASSWORD_BCRYPT; + } + + return 0; +} + static int php_password_salt_is_alphabet(const char *str, const int len) /* {{{ */ { int i = 0; @@ -149,6 +161,44 @@ static int php_password_make_salt(long length, int raw, char *ret TSRMLS_DC) /* } /* }}} */ +PHP_FUNCTION(password_needs_rehash) +{ + long new_algo = 0, algo = 0; + int hash_len; + char *hash; + HashTable *options = 0; + zval **option_buffer; + + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "sl|H", &hash, &hash_len, &new_algo, &options) == FAILURE) { + RETURN_NULL(); + } + algo = php_password_determine_algo(hash, hash_len); + + if (algo != new_algo) { + RETURN_TRUE; + } + + switch (algo) { + case PHP_PASSWORD_BCRYPT: + { + int newCost = PHP_PASSWORD_BCRYPT_COST, cost = 0; + + if (options && zend_symtable_find(options, "cost", 5, (void **) &option_buffer) == SUCCESS) { + convert_to_long_ex(option_buffer); + newCost = Z_LVAL_PP(option_buffer); + zval_ptr_dtor(option_buffer); + } + + sscanf(hash, "$2y$%d$", &cost); + if (cost != newCost) { + RETURN_TRUE; + } + } + break; + } + RETURN_FALSE; +} + /* {{{ proto boolean password_make_salt(string password, string hash) Verify a hash created using crypt() or password_hash() */ PHP_FUNCTION(password_verify) diff --git a/ext/standard/php_password.h b/ext/standard/php_password.h index 57c6b8878580b..45e6849936821 100644 --- a/ext/standard/php_password.h +++ b/ext/standard/php_password.h @@ -24,6 +24,7 @@ PHP_FUNCTION(password_hash); PHP_FUNCTION(password_verify); PHP_FUNCTION(password_make_salt); +PHP_FUNCTION(password_needs_rehash); PHP_MINIT_FUNCTION(password); From db86d54446c461eab518225645889abc509db034 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Thu, 5 Jul 2012 17:31:40 -0400 Subject: [PATCH 22/44] Fix issue with int vs long parameter --- ext/standard/password.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index 9bfb02358436c..6da656c5afed9 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -266,7 +266,8 @@ Hash a password */ PHP_FUNCTION(password_hash) { char *hash_format, *hash, *salt, *password, *result; - int algo = 0, salt_len = 0, required_salt_len = 0, hash_format_len, password_len; + long algo = 0; + int salt_len = 0, required_salt_len = 0, hash_format_len, password_len; HashTable *options = 0; zval **option_buffer; @@ -297,7 +298,7 @@ PHP_FUNCTION(password_hash) } break; default: - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unknown password hashing algorithm: %d", algo); + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unknown password hashing algorithm: %ld", algo); RETURN_NULL(); } From ee7e7998410c8fd5bd2183b1af375622f0ca8e02 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Thu, 5 Jul 2012 17:46:33 -0400 Subject: [PATCH 23/44] Implement password_get_info() function --- ext/standard/basic_functions.c | 4 ++++ ext/standard/password.c | 32 ++++++++++++++++++++++++++++++++ ext/standard/php_password.h | 1 + 3 files changed, 37 insertions(+) diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c index bf6f9b0b583c5..e6500dd66bdf5 100644 --- a/ext/standard/basic_functions.c +++ b/ext/standard/basic_functions.c @@ -1872,6 +1872,9 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_password_hash, 0, 0, 1) ZEND_ARG_INFO(0, algo) ZEND_ARG_INFO(0, options) ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_INFO_EX(arginfo_password_get_info, 0, 0, 1) + ZEND_ARG_INFO(0, hash) +ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_INFO_EX(arginfo_password_needs_rehash, 0, 0, 1) ZEND_ARG_INFO(0, hash) ZEND_ARG_INFO(0, algo) @@ -2901,6 +2904,7 @@ const zend_function_entry basic_functions[] = { /* {{{ */ PHP_FE(base64_encode, arginfo_base64_encode) PHP_FE(password_hash, arginfo_password_hash) + PHP_FE(password_get_info, arginfo_password_get_info) PHP_FE(password_needs_rehash, arginfo_password_needs_rehash) PHP_FE(password_verify, arginfo_password_verify) PHP_FE(password_make_salt, arginfo_password_make_salt) diff --git a/ext/standard/password.c b/ext/standard/password.c index 6da656c5afed9..9be6f8c366fd6 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -161,6 +161,38 @@ static int php_password_make_salt(long length, int raw, char *ret TSRMLS_DC) /* } /* }}} */ +PHP_FUNCTION(password_get_info) +{ + long algo; + int hash_len; + char *hash; + zval *options; + + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &hash, &hash_len) == FAILURE) { + RETURN_NULL(); + } + + ALLOC_INIT_ZVAL(options); + array_init(options); + + algo = php_password_determine_algo(hash, hash_len); + + switch (algo) { + case PHP_PASSWORD_BCRYPT: + { + long cost = PHP_PASSWORD_BCRYPT_COST; + sscanf(hash, "$2y$%ld$", &cost); + add_assoc_long(options, "cost", cost); + } + break; + } + + array_init(return_value); + + add_assoc_long(return_value, "algo", algo); + add_assoc_zval(return_value, "options", options); +} + PHP_FUNCTION(password_needs_rehash) { long new_algo = 0, algo = 0; diff --git a/ext/standard/php_password.h b/ext/standard/php_password.h index 45e6849936821..90e4d89bc480c 100644 --- a/ext/standard/php_password.h +++ b/ext/standard/php_password.h @@ -25,6 +25,7 @@ PHP_FUNCTION(password_hash); PHP_FUNCTION(password_verify); PHP_FUNCTION(password_make_salt); PHP_FUNCTION(password_needs_rehash); +PHP_FUNCTION(password_get_info); PHP_MINIT_FUNCTION(password); From 9d3630b5dc8fa066dc4212ead2fffc8635f5bc0a Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Thu, 5 Jul 2012 17:58:19 -0400 Subject: [PATCH 24/44] Cleanup whitespace issues --- ext/standard/password.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index 9be6f8c366fd6..2f1ebb5c59d6a 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -168,9 +168,9 @@ PHP_FUNCTION(password_get_info) char *hash; zval *options; - if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &hash, &hash_len) == FAILURE) { - RETURN_NULL(); - } + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &hash, &hash_len) == FAILURE) { + RETURN_NULL(); + } ALLOC_INIT_ZVAL(options); array_init(options); @@ -202,8 +202,8 @@ PHP_FUNCTION(password_needs_rehash) zval **option_buffer; if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "sl|H", &hash, &hash_len, &new_algo, &options) == FAILURE) { - RETURN_NULL(); - } + RETURN_NULL(); + } algo = php_password_determine_algo(hash, hash_len); if (algo != new_algo) { From 707c9073b595a75447fbc25e01e7804293fad9b7 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Wed, 11 Jul 2012 22:15:56 -0400 Subject: [PATCH 25/44] Switch second parameter to password_make_salt to be a flag --- ext/standard/password.c | 47 ++++++++++++------- ext/standard/php_password.h | 3 ++ .../tests/password/password_make_salt.phpt | 12 ++--- .../password/password_make_salt_error.phpt | 4 ++ 4 files changed, 43 insertions(+), 23 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index 2f1ebb5c59d6a..2e5d62acecaea 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -39,6 +39,10 @@ PHP_MINIT_FUNCTION(password) /* {{{ */ { REGISTER_LONG_CONSTANT("PASSWORD_DEFAULT", PHP_PASSWORD_DEFAULT, CONST_CS | CONST_PERSISTENT); REGISTER_LONG_CONSTANT("PASSWORD_BCRYPT", PHP_PASSWORD_BCRYPT, CONST_CS | CONST_PERSISTENT); + + REGISTER_LONG_CONSTANT("PASSWORD_SALT_RAW", PHP_PASSWORD_SALT_RAW, CONST_CS | CONST_PERSISTENT); + REGISTER_LONG_CONSTANT("PASSWORD_SALT_BCRYPT", PHP_PASSWORD_SALT_BCRYPT, CONST_CS | CONST_PERSISTENT); + return SUCCESS; } /* }}} */ @@ -55,15 +59,18 @@ static long php_password_determine_algo(const char *hash, const int len) return 0; } -static int php_password_salt_is_alphabet(const char *str, const int len) /* {{{ */ +static int php_password_salt_is_alphabet(const char *str, const int len, const int salt_type) /* {{{ */ { int i = 0; - for (i = 0; i < len; i++) { - if (!((str[i] >= 'A' && str[i] <= 'Z') || (str[i] >= 'a' && str[i] <= 'z') || (str[i] >= '0' && str[i] <= '9') || str[i] == '.' || str[i] == '/')) { - return 0; + if (salt_type == PHP_PASSWORD_SALT_BCRYPT) { + for (i = 0; i < len; i++) { + if (!((str[i] >= 'A' && str[i] <= 'Z') || (str[i] >= 'a' && str[i] <= 'z') || (str[i] >= '0' && str[i] <= '9') || str[i] == '.' || str[i] == '/')) { + return 0; + } } } + return 1; } /* }}} */ @@ -90,20 +97,23 @@ static int php_password_salt_to64(const char *str, const int str_len, const int #define PHP_PASSWORD_FUNCTION_EXISTS(func, func_len) (zend_hash_find(EG(function_table), (func), (func_len) + 1, (void **) &func_ptr) == SUCCESS && func_ptr->type == ZEND_INTERNAL_FUNCTION && func_ptr->internal_function.handler != zif_display_disabled_function) -static int php_password_make_salt(long length, int raw, char *ret TSRMLS_DC) /* {{{ */ +static int php_password_make_salt(long length, int salt_type, char *ret TSRMLS_DC) /* {{{ */ { int buffer_valid = 0; long i, raw_length; char *buffer; - if (raw) { + if (salt_type == PHP_PASSWORD_SALT_RAW) { raw_length = length; - } else { + } else if (salt_type == PHP_PASSWORD_SALT_BCRYPT) { if (length > (LONG_MAX / 3)) { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Length is too large to safely generate"); return FAILURE; } raw_length = length * 3 / 4 + 1; + } else { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unknown salt type paramter"); + return FAILURE; } buffer = (char *) safe_emalloc(raw_length, 1, 1); @@ -140,9 +150,7 @@ static int php_password_make_salt(long length, int raw, char *ret TSRMLS_DC) /* } } - if (raw) { - memcpy(ret, buffer, length); - } else { + if (salt_type == PHP_PASSWORD_SALT_BCRYPT) { char *result; result = safe_emalloc(length, 1, 1); if (php_password_salt_to64(buffer, raw_length, length, result) == FAILURE) { @@ -154,6 +162,9 @@ static int php_password_make_salt(long length, int raw, char *ret TSRMLS_DC) /* memcpy(ret, result, length); efree(result); } + } else { + /* PHP_PASSWORD_SALT_RAW */ + memcpy(ret, buffer, length); } efree(buffer); ret[length] = 0; @@ -266,14 +277,13 @@ PHP_FUNCTION(password_verify) } /* }}} */ -/* {{{ proto string password_make_salt(int length, boolean raw_output = false) +/* {{{ proto string password_make_salt(int length, int salt_type = PASSWORD_SALT_BCRYPT) Make a new random salt */ PHP_FUNCTION(password_make_salt) { char *salt; - long length = 0; - zend_bool raw_output = 0; - if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "l|b", &length, &raw_output) == FAILURE) { + long length = 0, salt_type = 0; + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "l|l", &length, &salt_type) == FAILURE) { RETURN_NULL(); } if (length <= 0) { @@ -284,8 +294,11 @@ PHP_FUNCTION(password_make_salt) RETURN_NULL(); } + if (!salt_type) { + salt_type = PHP_PASSWORD_SALT_BCRYPT; + } salt = safe_emalloc(length, 1, 1); - if (php_password_make_salt(length, (int) raw_output, salt TSRMLS_CC) == FAILURE) { + if (php_password_make_salt(length, (int) salt_type, salt TSRMLS_CC) == FAILURE) { efree(salt); RETURN_FALSE; } @@ -363,7 +376,7 @@ PHP_FUNCTION(password_hash) zval_ptr_dtor(option_buffer); php_error_docref(NULL TSRMLS_CC, E_WARNING, "Provided salt is too short: %d expecting %d", buffer_len, required_salt_len); RETURN_NULL(); - } else if (0 == php_password_salt_is_alphabet(buffer, buffer_len)) { + } else if (0 == php_password_salt_is_alphabet(buffer, buffer_len, PHP_PASSWORD_SALT_BCRYPT)) { salt = emalloc(required_salt_len + 1); if (php_password_salt_to64(buffer, buffer_len, required_salt_len, salt) == FAILURE) { efree(hash_format); @@ -381,7 +394,7 @@ PHP_FUNCTION(password_hash) zval_ptr_dtor(option_buffer); } else { salt = emalloc(required_salt_len + 1); - if (php_password_make_salt((long) required_salt_len, 0, salt TSRMLS_CC) == FAILURE) { + if (php_password_make_salt((long) required_salt_len, PHP_PASSWORD_SALT_BCRYPT, salt TSRMLS_CC) == FAILURE) { efree(hash_format); efree(salt); RETURN_FALSE; diff --git a/ext/standard/php_password.h b/ext/standard/php_password.h index 90e4d89bc480c..8211ae175331f 100644 --- a/ext/standard/php_password.h +++ b/ext/standard/php_password.h @@ -32,6 +32,9 @@ PHP_MINIT_FUNCTION(password); #define PHP_PASSWORD_DEFAULT 1 #define PHP_PASSWORD_BCRYPT 1 +#define PHP_PASSWORD_SALT_RAW 1 +#define PHP_PASSWORD_SALT_BCRYPT 2 + #define PHP_PASSWORD_BCRYPT_COST 10 #endif diff --git a/ext/standard/tests/password/password_make_salt.phpt b/ext/standard/tests/password/password_make_salt.phpt index 63b56f8544657..c7aa51455ed84 100644 --- a/ext/standard/tests/password/password_make_salt.phpt +++ b/ext/standard/tests/password/password_make_salt.phpt @@ -7,14 +7,14 @@ echo strlen(password_make_salt(1)) . "\n"; echo strlen(password_make_salt(2)) . "\n"; echo strlen(password_make_salt(3)) . "\n"; echo strlen(password_make_salt(4)) . "\n"; -echo strlen(password_make_salt(5)) . "\n"; +echo strlen(password_make_salt(5, PASSWORD_SALT_BCRYPT)) . "\n"; echo "\n"; -echo strlen(password_make_salt(1, true)) . "\n"; -echo strlen(password_make_salt(2, true)) . "\n"; -echo strlen(password_make_salt(3, true)) . "\n"; -echo strlen(password_make_salt(4, true)) . "\n"; -echo strlen(password_make_salt(5, true)) . "\n"; +echo strlen(password_make_salt(1, PASSWORD_SALT_RAW)) . "\n"; +echo strlen(password_make_salt(2, PASSWORD_SALT_RAW)) . "\n"; +echo strlen(password_make_salt(3, PASSWORD_SALT_RAW)) . "\n"; +echo strlen(password_make_salt(4, PASSWORD_SALT_RAW)) . "\n"; +echo strlen(password_make_salt(5, PASSWORD_SALT_RAW)) . "\n"; echo "\n"; $a = password_make_salt(32); diff --git a/ext/standard/tests/password/password_make_salt_error.phpt b/ext/standard/tests/password/password_make_salt_error.phpt index 4a1d5e29c9837..92df53ad0fef3 100644 --- a/ext/standard/tests/password/password_make_salt_error.phpt +++ b/ext/standard/tests/password/password_make_salt_error.phpt @@ -14,6 +14,8 @@ var_dump(password_make_salt(PHP_INT_MAX)); var_dump(password_make_salt(floor(PHP_INT_MAX / 2.9))); +var_dump(password_make_salt(5, 999)); + ?> --EXPECTF-- Warning: password_make_salt() expects at least 1 parameter, 0 given in %s on line %d @@ -31,3 +33,5 @@ NULL Warning: password_make_salt(): Length is too large to safely generate in %s on line %d NULL +Warning: password_make_salt(): Unknown salt type paramter in %s on line %d +bool(false) From e05413ca594ff10fd93d40429cb598c2e109edf4 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Tue, 28 Aug 2012 11:24:33 -0400 Subject: [PATCH 26/44] Remove password_make_salt() from the implementation --- ext/standard/basic_functions.c | 6 --- ext/standard/password.c | 34 ---------------- ext/standard/php_password.h | 1 - .../tests/password/password_make_salt.phpt | 40 ------------------- .../password/password_make_salt_error.phpt | 37 ----------------- 5 files changed, 118 deletions(-) delete mode 100644 ext/standard/tests/password/password_make_salt.phpt delete mode 100644 ext/standard/tests/password/password_make_salt_error.phpt diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c index e6b155979fbcb..1f1b3d366db95 100644 --- a/ext/standard/basic_functions.c +++ b/ext/standard/basic_functions.c @@ -1884,10 +1884,6 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_password_verify, 0, 0, 2) ZEND_ARG_INFO(0, password) ZEND_ARG_INFO(0, hash) ZEND_END_ARG_INFO() -ZEND_BEGIN_ARG_INFO_EX(arginfo_password_make_salt, 0, 0, 1) - ZEND_ARG_INFO(0, length) - ZEND_ARG_INFO(0, raw_output) -ZEND_END_ARG_INFO() /* }}} */ /* {{{ proc_open.c */ #ifdef PHP_CAN_SUPPORT_PROC_OPEN @@ -2907,8 +2903,6 @@ const zend_function_entry basic_functions[] = { /* {{{ */ PHP_FE(password_get_info, arginfo_password_get_info) PHP_FE(password_needs_rehash, arginfo_password_needs_rehash) PHP_FE(password_verify, arginfo_password_verify) - PHP_FE(password_make_salt, arginfo_password_make_salt) - PHP_FE(convert_uuencode, arginfo_convert_uuencode) PHP_FE(convert_uudecode, arginfo_convert_uudecode) diff --git a/ext/standard/password.c b/ext/standard/password.c index 2e5d62acecaea..4f8ef5dcab7ca 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -40,9 +40,6 @@ PHP_MINIT_FUNCTION(password) /* {{{ */ REGISTER_LONG_CONSTANT("PASSWORD_DEFAULT", PHP_PASSWORD_DEFAULT, CONST_CS | CONST_PERSISTENT); REGISTER_LONG_CONSTANT("PASSWORD_BCRYPT", PHP_PASSWORD_BCRYPT, CONST_CS | CONST_PERSISTENT); - REGISTER_LONG_CONSTANT("PASSWORD_SALT_RAW", PHP_PASSWORD_SALT_RAW, CONST_CS | CONST_PERSISTENT); - REGISTER_LONG_CONSTANT("PASSWORD_SALT_BCRYPT", PHP_PASSWORD_SALT_BCRYPT, CONST_CS | CONST_PERSISTENT); - return SUCCESS; } /* }}} */ @@ -95,8 +92,6 @@ static int php_password_salt_to64(const char *str, const int str_len, const int } /* }}} */ -#define PHP_PASSWORD_FUNCTION_EXISTS(func, func_len) (zend_hash_find(EG(function_table), (func), (func_len) + 1, (void **) &func_ptr) == SUCCESS && func_ptr->type == ZEND_INTERNAL_FUNCTION && func_ptr->internal_function.handler != zif_display_disabled_function) - static int php_password_make_salt(long length, int salt_type, char *ret TSRMLS_DC) /* {{{ */ { int buffer_valid = 0; @@ -277,35 +272,6 @@ PHP_FUNCTION(password_verify) } /* }}} */ -/* {{{ proto string password_make_salt(int length, int salt_type = PASSWORD_SALT_BCRYPT) -Make a new random salt */ -PHP_FUNCTION(password_make_salt) -{ - char *salt; - long length = 0, salt_type = 0; - if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "l|l", &length, &salt_type) == FAILURE) { - RETURN_NULL(); - } - if (length <= 0) { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Length cannot be less than or equal zero: %ld", length); - RETURN_NULL(); - } else if (length > (LONG_MAX / 3)) { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Length is too large to safely generate"); - RETURN_NULL(); - } - - if (!salt_type) { - salt_type = PHP_PASSWORD_SALT_BCRYPT; - } - salt = safe_emalloc(length, 1, 1); - if (php_password_make_salt(length, (int) salt_type, salt TSRMLS_CC) == FAILURE) { - efree(salt); - RETURN_FALSE; - } - RETURN_STRINGL(salt, length, 0); -} -/* }}} */ - /* {{{ proto string password_hash(string password, int algo, array options = array()) Hash a password */ PHP_FUNCTION(password_hash) diff --git a/ext/standard/php_password.h b/ext/standard/php_password.h index 8211ae175331f..d99c061c008bf 100644 --- a/ext/standard/php_password.h +++ b/ext/standard/php_password.h @@ -23,7 +23,6 @@ PHP_FUNCTION(password_hash); PHP_FUNCTION(password_verify); -PHP_FUNCTION(password_make_salt); PHP_FUNCTION(password_needs_rehash); PHP_FUNCTION(password_get_info); diff --git a/ext/standard/tests/password/password_make_salt.phpt b/ext/standard/tests/password/password_make_salt.phpt deleted file mode 100644 index c7aa51455ed84..0000000000000 --- a/ext/standard/tests/password/password_make_salt.phpt +++ /dev/null @@ -1,40 +0,0 @@ ---TEST-- -Test normal operation of password_make_salt() ---FILE-- - ---EXPECT-- -1 -2 -3 -4 -5 - -1 -2 -3 -4 -5 - -bool(true) -OK! diff --git a/ext/standard/tests/password/password_make_salt_error.phpt b/ext/standard/tests/password/password_make_salt_error.phpt deleted file mode 100644 index 92df53ad0fef3..0000000000000 --- a/ext/standard/tests/password/password_make_salt_error.phpt +++ /dev/null @@ -1,37 +0,0 @@ ---TEST-- -Test error operation of password_make_salt() ---FILE-- - ---EXPECTF-- -Warning: password_make_salt() expects at least 1 parameter, 0 given in %s on line %d -NULL - -Warning: password_make_salt() expects parameter 1 to be long, string given in %s on line %d -NULL - -Warning: password_make_salt(): Length cannot be less than or equal zero: -1 in %s on line %d -NULL - -Warning: password_make_salt(): Length is too large to safely generate in %s on line %d -NULL - -Warning: password_make_salt(): Length is too large to safely generate in %s on line %d -NULL - -Warning: password_make_salt(): Unknown salt type paramter in %s on line %d -bool(false) From db41f9fe60d863041fb53a273c2f64b6925f5ad0 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Tue, 4 Sep 2012 11:34:00 -0400 Subject: [PATCH 27/44] Refactoring to use size_t instead of int most places --- ext/standard/password.c | 150 +++++++++++++++++++++--------------- ext/standard/php_password.h | 3 - 2 files changed, 90 insertions(+), 63 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index 4f8ef5dcab7ca..d3dc45742864c 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -44,7 +44,17 @@ PHP_MINIT_FUNCTION(password) /* {{{ */ } /* }}} */ -static long php_password_determine_algo(const char *hash, const int len) +static char* php_password_get_algo_name(const int algo) +{ + switch (algo) { + case PHP_PASSWORD_BCRYPT: + return "bcrypt"; + default: + return "unknown"; + } +} + +static int php_password_determine_algo(const char *hash, const size_t len) { if (len < 3) { return 0; @@ -56,27 +66,33 @@ static long php_password_determine_algo(const char *hash, const int len) return 0; } -static int php_password_salt_is_alphabet(const char *str, const int len, const int salt_type) /* {{{ */ +static int php_password_salt_is_alphabet(const char *str, const size_t len) /* {{{ */ { - int i = 0; + size_t i = 0; - if (salt_type == PHP_PASSWORD_SALT_BCRYPT) { - for (i = 0; i < len; i++) { - if (!((str[i] >= 'A' && str[i] <= 'Z') || (str[i] >= 'a' && str[i] <= 'z') || (str[i] >= '0' && str[i] <= '9') || str[i] == '.' || str[i] == '/')) { - return 0; - } + for (i = 0; i < len; i++) { + if (!((str[i] >= 'A' && str[i] <= 'Z') || (str[i] >= 'a' && str[i] <= 'z') || (str[i] >= '0' && str[i] <= '9') || str[i] == '.' || str[i] == '/')) { + return 0; } } - return 1; } /* }}} */ -static int php_password_salt_to64(const char *str, const int str_len, const int out_len, char *ret) /* {{{ */ +static int php_password_salt_to64(const char *str, const size_t str_len, const size_t out_len, char *ret) /* {{{ */ { - int pos = 0; + size_t pos = 0; + size_t ret_len = 0; unsigned char *buffer; - buffer = php_base64_encode((unsigned char*) str, str_len, NULL); + if ((int) str_len < 0) { + return FAILURE; + } + buffer = php_base64_encode((unsigned char*) str, (int) str_len, (int*) &ret_len); + if (ret_len < out_len) { + /* Too short of an encoded string generated */ + efree(buffer); + return FAILURE; + } for (pos = 0; pos < out_len; pos++) { if (buffer[pos] == '+') { ret[pos] = '.'; @@ -92,30 +108,26 @@ static int php_password_salt_to64(const char *str, const int str_len, const int } /* }}} */ -static int php_password_make_salt(long length, int salt_type, char *ret TSRMLS_DC) /* {{{ */ +static int php_password_make_salt(size_t length, char *ret TSRMLS_DC) /* {{{ */ { int buffer_valid = 0; - long i, raw_length; + size_t i, raw_length; char *buffer; + char *result; - if (salt_type == PHP_PASSWORD_SALT_RAW) { - raw_length = length; - } else if (salt_type == PHP_PASSWORD_SALT_BCRYPT) { - if (length > (LONG_MAX / 3)) { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Length is too large to safely generate"); - return FAILURE; - } - raw_length = length * 3 / 4 + 1; - } else { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unknown salt type paramter"); + if (length > (INT_MAX / 3)) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Length is too large to safely generate"); return FAILURE; } + + raw_length = length * 3 / 4 + 1; + buffer = (char *) safe_emalloc(raw_length, 1, 1); #if PHP_WIN32 { BYTE *iv_b = (BYTE *) buffer; - if (php_win32_get_random_bytes(iv_b, (size_t) raw_length) == SUCCESS) { + if (php_win32_get_random_bytes(iv_b, raw_length) == SUCCESS) { buffer_valid = 1; } } @@ -130,11 +142,11 @@ static int php_password_make_salt(long length, int salt_type, char *ret TSRMLS_D if (n < 0) { break; } - read_bytes += n; + read_bytes += (size_t) n; } close(fd); } - if (read_bytes == raw_length) { + if (read_bytes >= raw_length) { buffer_valid = 1; } } @@ -145,22 +157,16 @@ static int php_password_make_salt(long length, int salt_type, char *ret TSRMLS_D } } - if (salt_type == PHP_PASSWORD_SALT_BCRYPT) { - char *result; - result = safe_emalloc(length, 1, 1); - if (php_password_salt_to64(buffer, raw_length, length, result) == FAILURE) { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Generated salt too short"); - efree(buffer); - efree(result); - return FAILURE; - } else { - memcpy(ret, result, length); - efree(result); - } + result = safe_emalloc(length, 1, 1); + if (php_password_salt_to64(buffer, raw_length, length, result) == FAILURE) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Generated salt too short"); + efree(buffer); + efree(result); + return FAILURE; } else { - /* PHP_PASSWORD_SALT_RAW */ - memcpy(ret, buffer, length); + memcpy(ret, result, (int) length); } + efree(result); efree(buffer); ret[length] = 0; return SUCCESS; @@ -171,17 +177,23 @@ PHP_FUNCTION(password_get_info) { long algo; int hash_len; - char *hash; + char *hash, *algoName; zval *options; if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &hash, &hash_len) == FAILURE) { RETURN_NULL(); } + if (hash_len < 0 || (size_t) hash_len < 0) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Supplied Password Hash Too Long To Safely Identify"); + RETURN_FALSE; + } + ALLOC_INIT_ZVAL(options); array_init(options); - algo = php_password_determine_algo(hash, hash_len); + algo = php_password_determine_algo(hash, (size_t) hash_len); + algoName = php_password_get_algo_name(algo); switch (algo) { case PHP_PASSWORD_BCRYPT: @@ -196,6 +208,7 @@ PHP_FUNCTION(password_get_info) array_init(return_value); add_assoc_long(return_value, "algo", algo); + add_assoc_string(return_value, "algoName", algoName, 1); add_assoc_zval(return_value, "options", options); } @@ -210,7 +223,13 @@ PHP_FUNCTION(password_needs_rehash) if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "sl|H", &hash, &hash_len, &new_algo, &options) == FAILURE) { RETURN_NULL(); } - algo = php_password_determine_algo(hash, hash_len); + + if (hash_len < 0) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Supplied Password Hash Too Long To Safely Identify"); + RETURN_FALSE; + } + + algo = php_password_determine_algo(hash, (size_t) hash_len); if (algo != new_algo) { RETURN_TRUE; @@ -252,7 +271,7 @@ PHP_FUNCTION(password_verify) RETURN_FALSE; } - if (strlen(ret) != hash_len) { + if (strlen(ret) != hash_len || hash_len < 13) { efree(ret); RETURN_FALSE; } @@ -278,7 +297,8 @@ PHP_FUNCTION(password_hash) { char *hash_format, *hash, *salt, *password, *result; long algo = 0; - int salt_len = 0, required_salt_len = 0, hash_format_len, password_len; + int password_len = 0, hash_len; + size_t salt_len = 0, required_salt_len = 0, hash_format_len; HashTable *options = 0; zval **option_buffer; @@ -289,7 +309,7 @@ PHP_FUNCTION(password_hash) switch (algo) { case PHP_PASSWORD_BCRYPT: { - int cost = PHP_PASSWORD_BCRYPT_COST; + long cost = PHP_PASSWORD_BCRYPT_COST; if (options && zend_symtable_find(options, "cost", 5, (void **) &option_buffer) == SUCCESS) { convert_to_long_ex(option_buffer); @@ -298,13 +318,13 @@ PHP_FUNCTION(password_hash) } if (cost < 4 || cost > 31) { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Invalid bcrypt cost parameter specified: %d", cost); + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Invalid bcrypt cost parameter specified: %ld", cost); RETURN_NULL(); } required_salt_len = 22; hash_format = emalloc(8); - sprintf(hash_format, "$2y$%02d$", cost); + sprintf(hash_format, "$2y$%02ld$", cost); hash_format_len = 7; } break; @@ -315,7 +335,8 @@ PHP_FUNCTION(password_hash) if (options && zend_symtable_find(options, "salt", 5, (void**) &option_buffer) == SUCCESS) { char *buffer; - int buffer_len; + int buffer_len_int; + size_t buffer_len; switch (Z_TYPE_PP(option_buffer)) { case IS_NULL: case IS_STRING: @@ -326,7 +347,13 @@ PHP_FUNCTION(password_hash) convert_to_string_ex(option_buffer); if (Z_TYPE_PP(option_buffer) == IS_STRING) { buffer = Z_STRVAL_PP(option_buffer); - buffer_len = Z_STRLEN_PP(option_buffer); + buffer_len_int = Z_STRLEN_PP(option_buffer); + if (buffer_len_int < 0) { + zval_ptr_dtor(option_buffer); + efree(hash_format); + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Supplied salt is too long"); + } + buffer_len = (size_t) buffer_len_int; break; } case IS_RESOURCE: @@ -340,27 +367,27 @@ PHP_FUNCTION(password_hash) if (buffer_len < required_salt_len) { efree(hash_format); zval_ptr_dtor(option_buffer); - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Provided salt is too short: %d expecting %d", buffer_len, required_salt_len); + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Provided salt is too short: %lu expecting %lu", (unsigned long) buffer_len, (unsigned long) required_salt_len); RETURN_NULL(); - } else if (0 == php_password_salt_is_alphabet(buffer, buffer_len, PHP_PASSWORD_SALT_BCRYPT)) { - salt = emalloc(required_salt_len + 1); + } else if (0 == php_password_salt_is_alphabet(buffer, buffer_len)) { + salt = safe_emalloc(required_salt_len, 1, 1); if (php_password_salt_to64(buffer, buffer_len, required_salt_len, salt) == FAILURE) { efree(hash_format); efree(salt); zval_ptr_dtor(option_buffer); - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Provided salt is too short: %d", salt_len); + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Provided salt is too short: %lu", (unsigned long) buffer_len); RETURN_NULL(); } salt_len = required_salt_len; } else { - salt = emalloc(required_salt_len + 1); - memcpy(salt, buffer, required_salt_len); + salt = safe_emalloc(required_salt_len, 1, 1); + memcpy(salt, buffer, (int) required_salt_len); salt_len = required_salt_len; } zval_ptr_dtor(option_buffer); } else { - salt = emalloc(required_salt_len + 1); - if (php_password_make_salt((long) required_salt_len, PHP_PASSWORD_SALT_BCRYPT, salt TSRMLS_CC) == FAILURE) { + salt = safe_emalloc(required_salt_len, 1, 1); + if (php_password_make_salt(required_salt_len, salt TSRMLS_CC) == FAILURE) { efree(hash_format); efree(salt); RETURN_FALSE; @@ -377,7 +404,10 @@ PHP_FUNCTION(password_hash) efree(hash_format); efree(salt); - if (php_crypt(password, password_len, hash, hash_format_len + salt_len, &result) == FAILURE) { + /* This cast is safe, since both values are defined here in code and cannot overflow */ + hash_len = (int) (hash_format_len + salt_len); + + if (php_crypt(password, password_len, hash, hash_len, &result) == FAILURE) { efree(hash); RETURN_FALSE; } diff --git a/ext/standard/php_password.h b/ext/standard/php_password.h index d99c061c008bf..db7747a3db6b9 100644 --- a/ext/standard/php_password.h +++ b/ext/standard/php_password.h @@ -31,9 +31,6 @@ PHP_MINIT_FUNCTION(password); #define PHP_PASSWORD_DEFAULT 1 #define PHP_PASSWORD_BCRYPT 1 -#define PHP_PASSWORD_SALT_RAW 1 -#define PHP_PASSWORD_SALT_BCRYPT 2 - #define PHP_PASSWORD_BCRYPT_COST 10 #endif From e8b7f5b35da46a2bc414c922e8e1a7093d963899 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Wed, 12 Sep 2012 11:21:08 -0400 Subject: [PATCH 28/44] Add tests for password_get_info and password_needs_rehash --- .../tests/password/password_get_info.phpt | 58 +++++++++++++++++++ .../password/password_get_info_error.phpt | 17 ++++++ .../tests/password/password_needs_rehash.phpt | 39 +++++++++++++ .../password/password_needs_rehash_error.phpt | 33 +++++++++++ 4 files changed, 147 insertions(+) create mode 100644 ext/standard/tests/password/password_get_info.phpt create mode 100644 ext/standard/tests/password/password_get_info_error.phpt create mode 100644 ext/standard/tests/password/password_needs_rehash.phpt create mode 100644 ext/standard/tests/password/password_needs_rehash_error.phpt diff --git a/ext/standard/tests/password/password_get_info.phpt b/ext/standard/tests/password/password_get_info.phpt new file mode 100644 index 0000000000000..4c8dc04ff802e --- /dev/null +++ b/ext/standard/tests/password/password_get_info.phpt @@ -0,0 +1,58 @@ +--TEST-- +Test normal operation of password_get_info() +--FILE-- + +--EXPECT-- +array(3) { + ["algo"]=> + int(1) + ["algoName"]=> + string(6) "bcrypt" + ["options"]=> + array(1) { + ["cost"]=> + int(10) + } +} +array(3) { + ["algo"]=> + int(1) + ["algoName"]=> + string(6) "bcrypt" + ["options"]=> + array(1) { + ["cost"]=> + int(11) + } +} +array(3) { + ["algo"]=> + int(0) + ["algoName"]=> + string(7) "unknown" + ["options"]=> + array(0) { + } +} +array(3) { + ["algo"]=> + int(0) + ["algoName"]=> + string(7) "unknown" + ["options"]=> + array(0) { + } +} +OK! diff --git a/ext/standard/tests/password/password_get_info_error.phpt b/ext/standard/tests/password/password_get_info_error.phpt new file mode 100644 index 0000000000000..af676744c8b30 --- /dev/null +++ b/ext/standard/tests/password/password_get_info_error.phpt @@ -0,0 +1,17 @@ +--TEST-- +Test error operation of password_get_info() +--FILE-- + +--EXPECTF-- +Warning: password_get_info() expects exactly 1 parameter, 0 given in %s on line %d +NULL + +Warning: password_get_info() expects parameter 1 to be string, array given in %s on line %d +NULL +OK! diff --git a/ext/standard/tests/password/password_needs_rehash.phpt b/ext/standard/tests/password/password_needs_rehash.phpt new file mode 100644 index 0000000000000..0c03d88b4d440 --- /dev/null +++ b/ext/standard/tests/password/password_needs_rehash.phpt @@ -0,0 +1,39 @@ +--TEST-- +Test normal operation of password_needs_rehash() +--FILE-- + 10))); + +// Valid with cost the same, additional params +var_dump(password_needs_rehash('$2y$10$MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR.oP7XOR9HD/vjUuOj100y', PASSWORD_BCRYPT, array('cost' => 10, 'foo' => 3))); + +// Invalid, different (lower) cost +var_dump(password_needs_rehash('$2y$10$MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR.oP7XOR9HD/vjUuOj100y', PASSWORD_BCRYPT, array('cost' => 09))); + +// Invalid, different (higher) cost +var_dump(password_needs_rehash('$2y$10$MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR.oP7XOR9HD/vjUuOj100y', PASSWORD_BCRYPT, array('cost' => 11))); + +// Valid with cost the default (may need to be updated as the default cost increases) +var_dump(password_needs_rehash('$2y$10$MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR.oP7XOR9HD/vjUuOj100y', PASSWORD_BCRYPT)); + + +echo "OK!"; +?> +--EXPECT-- +bool(true) +bool(false) +bool(false) +bool(false) +bool(true) +bool(true) +bool(false) +OK! diff --git a/ext/standard/tests/password/password_needs_rehash_error.phpt b/ext/standard/tests/password/password_needs_rehash_error.phpt new file mode 100644 index 0000000000000..e25ef8db3f4dd --- /dev/null +++ b/ext/standard/tests/password/password_needs_rehash_error.phpt @@ -0,0 +1,33 @@ +--TEST-- +Test error operation of password_needs_rehash() +--FILE-- + +--EXPECTF-- +Warning: password_needs_rehash() expects at least 2 parameters, 0 given in %s on line %d +NULL + +Warning: password_needs_rehash() expects at least 2 parameters, 1 given in %s on line %d +NULL + +Warning: password_needs_rehash() expects parameter 2 to be long, string given in %s on line %d +NULL + +Warning: password_needs_rehash() expects parameter 1 to be string, array given in %s on line %d +NULL + +Warning: password_needs_rehash() expects parameter 3 to be array, string given in %s on line %d +NULL +OK! From e9a7bde829b3e43e2c61455752801e31ea88974f Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Wed, 12 Sep 2012 11:37:56 -0400 Subject: [PATCH 29/44] Switch test to using strict comparison for crypt fallback --- ext/standard/tests/password/password_hash.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/standard/tests/password/password_hash.phpt b/ext/standard/tests/password/password_hash.phpt index ff48b29b169a9..f59d3d5e488ab 100644 --- a/ext/standard/tests/password/password_hash.phpt +++ b/ext/standard/tests/password/password_hash.phpt @@ -8,7 +8,7 @@ var_dump(strlen(password_hash("foo", PASSWORD_BCRYPT))); $hash = password_hash("foo", PASSWORD_BCRYPT); -var_dump($hash == crypt("foo", $hash)); +var_dump($hash === crypt("foo", $hash)); var_dump(password_hash("rasmuslerdorf", PASSWORD_BCRYPT, array("cost" => 7, "salt" => "usesomesillystringforsalt"))); From ebe0bd5dee07bebd8444d9e7c28864ba17efeef8 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Wed, 12 Sep 2012 11:44:03 -0400 Subject: [PATCH 30/44] Remove bcrypt_cost ini entry from declaration --- main/main.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/main/main.c b/main/main.c index 2f40dc91b7df2..5eb9947fe7a5c 100644 --- a/main/main.c +++ b/main/main.c @@ -539,8 +539,6 @@ PHP_INI_BEGIN() STD_PHP_INI_ENTRY("error_append_string", NULL, PHP_INI_ALL, OnUpdateString, error_append_string, php_core_globals, core_globals) STD_PHP_INI_ENTRY("error_prepend_string", NULL, PHP_INI_ALL, OnUpdateString, error_prepend_string, php_core_globals, core_globals) - PHP_INI_ENTRY("password.bcrypt_cost", "11", PHP_INI_ALL, NULL) - PHP_INI_ENTRY("SMTP", "localhost",PHP_INI_ALL, NULL) PHP_INI_ENTRY("smtp_port", "25", PHP_INI_ALL, NULL) STD_PHP_INI_BOOLEAN("mail.add_x_header", "0", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateBool, mail_x_header, php_core_globals, core_globals) From 76f3295cdfd6a3106297352e73b9691084582211 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Wed, 12 Sep 2012 11:47:50 -0400 Subject: [PATCH 31/44] Expose PASSWORD_BCRYPT_DEFAULT_COST constant and update test to use it --- ext/standard/password.c | 2 ++ ext/standard/tests/password/password_needs_rehash.phpt | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index d3dc45742864c..9b1bb8cccaf5b 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -40,6 +40,8 @@ PHP_MINIT_FUNCTION(password) /* {{{ */ REGISTER_LONG_CONSTANT("PASSWORD_DEFAULT", PHP_PASSWORD_DEFAULT, CONST_CS | CONST_PERSISTENT); REGISTER_LONG_CONSTANT("PASSWORD_BCRYPT", PHP_PASSWORD_BCRYPT, CONST_CS | CONST_PERSISTENT); + REGISTER_LONG_CONSTANT("PASSWORD_BCRYPT_DEFAULT_COST", PHP_PASSWORD_BCRYPT_COST, CONST_CS | CONST_PERSISTENT); + return SUCCESS; } /* }}} */ diff --git a/ext/standard/tests/password/password_needs_rehash.phpt b/ext/standard/tests/password/password_needs_rehash.phpt index 0c03d88b4d440..2fc3983980165 100644 --- a/ext/standard/tests/password/password_needs_rehash.phpt +++ b/ext/standard/tests/password/password_needs_rehash.phpt @@ -22,9 +22,9 @@ var_dump(password_needs_rehash('$2y$10$MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR.oP7XOR9H // Invalid, different (higher) cost var_dump(password_needs_rehash('$2y$10$MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR.oP7XOR9HD/vjUuOj100y', PASSWORD_BCRYPT, array('cost' => 11))); -// Valid with cost the default (may need to be updated as the default cost increases) -var_dump(password_needs_rehash('$2y$10$MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR.oP7XOR9HD/vjUuOj100y', PASSWORD_BCRYPT)); - +// Valid with cost the default +$cost = str_pad(PASSWORD_BCRYPT_DEFAULT_COST, 2, '0', STR_PAD_LEFT); +var_dump(password_needs_rehash('$2y$'.$cost.'$MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR.oP7XOR9HD/vjUuOj100y', PASSWORD_BCRYPT)); echo "OK!"; ?> From 7161c3d2cfde54ce218f20d03684f2a58e1c7627 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Wed, 12 Sep 2012 11:56:12 -0400 Subject: [PATCH 32/44] Add news entry for password API --- NEWS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS b/NEWS index 1ee977974a58b..08045fc23a185 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,8 @@ PHP NEWS ?? ??? 201?, PHP 5.5.0 - General improvements: + . Add simplified password hashing API + (https://wiki.php.net/rfc/password_hash). (Anthony Ferrara) . Support list in foreach (https://wiki.php.net/rfc/foreachlist). (Laruence) . Implemented 'finally' keyword (https://wiki.php.net/rfc/finally). (Laruence) . Drop Windows XP and 2003 support. (Pierre) From 7ec80e1a139ca7f43c02728f3fe2424cef0138b6 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Wed, 12 Sep 2012 12:15:33 -0400 Subject: [PATCH 33/44] Fix incorrect arg info required param count for password_hash --- ext/standard/basic_functions.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c index ece64f375f4e9..cf2266c31dd85 100644 --- a/ext/standard/basic_functions.c +++ b/ext/standard/basic_functions.c @@ -1855,7 +1855,7 @@ ZEND_BEGIN_ARG_INFO(arginfo_getlastmod, 0) ZEND_END_ARG_INFO() /* }}} */ /* {{{ password.c */ -ZEND_BEGIN_ARG_INFO_EX(arginfo_password_hash, 0, 0, 1) +ZEND_BEGIN_ARG_INFO_EX(arginfo_password_hash, 0, 0, 2) ZEND_ARG_INFO(0, password) ZEND_ARG_INFO(0, algo) ZEND_ARG_INFO(0, options) From 83cfff4593bd3bd7791f32795e9b5bda446cd8e2 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Thu, 13 Sep 2012 10:32:54 -0400 Subject: [PATCH 34/44] Switch to using an ENUM for algorithms instead of a constant --- ext/standard/password.c | 38 ++++++++++++++++++++----------------- ext/standard/php_password.h | 8 ++++++-- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index 9b1bb8cccaf5b..0dd8fed645565 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -38,7 +38,7 @@ PHP_MINIT_FUNCTION(password) /* {{{ */ { REGISTER_LONG_CONSTANT("PASSWORD_DEFAULT", PHP_PASSWORD_DEFAULT, CONST_CS | CONST_PERSISTENT); - REGISTER_LONG_CONSTANT("PASSWORD_BCRYPT", PHP_PASSWORD_BCRYPT, CONST_CS | CONST_PERSISTENT); + REGISTER_LONG_CONSTANT("PASSWORD_BCRYPT", PASSWORD_BCRYPT, CONST_CS | CONST_PERSISTENT); REGISTER_LONG_CONSTANT("PASSWORD_BCRYPT_DEFAULT_COST", PHP_PASSWORD_BCRYPT_COST, CONST_CS | CONST_PERSISTENT); @@ -46,29 +46,26 @@ PHP_MINIT_FUNCTION(password) /* {{{ */ } /* }}} */ -static char* php_password_get_algo_name(const int algo) +static char* php_password_get_algo_name(const php_password_algos algo) { switch (algo) { - case PHP_PASSWORD_BCRYPT: + case PASSWORD_BCRYPT: return "bcrypt"; default: return "unknown"; } } -static int php_password_determine_algo(const char *hash, const size_t len) +static php_password_algos php_password_determine_algo(const char *hash, const size_t len) { - if (len < 3) { - return 0; - } - if (hash[0] == '$' && hash[1] == '2' && hash[2] == 'y' && len == 60) { - return PHP_PASSWORD_BCRYPT; + if (len > 3 && hash[0] == '$' && hash[1] == '2' && hash[2] == 'y' && len == 60) { + return PASSWORD_BCRYPT; } - return 0; + return PASSWORD_UNKNOWN; } -static int php_password_salt_is_alphabet(const char *str, const size_t len) /* {{{ */ +static zend_bool php_password_salt_is_alphabet(const char *str, const size_t len) /* {{{ */ { size_t i = 0; @@ -177,7 +174,7 @@ static int php_password_make_salt(size_t length, char *ret TSRMLS_DC) /* {{{ */ PHP_FUNCTION(password_get_info) { - long algo; + php_password_algos algo; int hash_len; char *hash, *algoName; zval *options; @@ -198,13 +195,16 @@ PHP_FUNCTION(password_get_info) algoName = php_password_get_algo_name(algo); switch (algo) { - case PHP_PASSWORD_BCRYPT: + case PASSWORD_BCRYPT: { long cost = PHP_PASSWORD_BCRYPT_COST; sscanf(hash, "$2y$%ld$", &cost); add_assoc_long(options, "cost", cost); } - break; + break; + case PASSWORD_UNKNOWN: + default: + break; } array_init(return_value); @@ -216,7 +216,8 @@ PHP_FUNCTION(password_get_info) PHP_FUNCTION(password_needs_rehash) { - long new_algo = 0, algo = 0; + long new_algo = 0; + php_password_algos algo; int hash_len; char *hash; HashTable *options = 0; @@ -238,7 +239,7 @@ PHP_FUNCTION(password_needs_rehash) } switch (algo) { - case PHP_PASSWORD_BCRYPT: + case PASSWORD_BCRYPT: { int newCost = PHP_PASSWORD_BCRYPT_COST, cost = 0; @@ -254,6 +255,9 @@ PHP_FUNCTION(password_needs_rehash) } } break; + case PASSWORD_UNKNOWN: + default: + break; } RETURN_FALSE; } @@ -309,7 +313,7 @@ PHP_FUNCTION(password_hash) } switch (algo) { - case PHP_PASSWORD_BCRYPT: + case PASSWORD_BCRYPT: { long cost = PHP_PASSWORD_BCRYPT_COST; diff --git a/ext/standard/php_password.h b/ext/standard/php_password.h index db7747a3db6b9..c812e2c492ae5 100644 --- a/ext/standard/php_password.h +++ b/ext/standard/php_password.h @@ -28,11 +28,15 @@ PHP_FUNCTION(password_get_info); PHP_MINIT_FUNCTION(password); -#define PHP_PASSWORD_DEFAULT 1 -#define PHP_PASSWORD_BCRYPT 1 +#define PHP_PASSWORD_DEFAULT PASSWORD_BCRYPT #define PHP_PASSWORD_BCRYPT_COST 10 +typedef enum { + PASSWORD_UNKNOWN, + PASSWORD_BCRYPT +} php_password_algos; + #endif From e034a46bdc36fb82957f5e503fa730776dfbba11 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Mon, 17 Sep 2012 10:52:07 -0400 Subject: [PATCH 35/44] A bunch of naming convention fixes. No functionality changes --- ext/standard/password.c | 40 +++++++++++++++++++------------------ ext/standard/php_password.h | 8 ++++---- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index 0dd8fed645565..6c2a9af3aa3ae 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -38,7 +38,7 @@ PHP_MINIT_FUNCTION(password) /* {{{ */ { REGISTER_LONG_CONSTANT("PASSWORD_DEFAULT", PHP_PASSWORD_DEFAULT, CONST_CS | CONST_PERSISTENT); - REGISTER_LONG_CONSTANT("PASSWORD_BCRYPT", PASSWORD_BCRYPT, CONST_CS | CONST_PERSISTENT); + REGISTER_LONG_CONSTANT("PASSWORD_BCRYPT", PHP_PASSWORD_BCRYPT, CONST_CS | CONST_PERSISTENT); REGISTER_LONG_CONSTANT("PASSWORD_BCRYPT_DEFAULT_COST", PHP_PASSWORD_BCRYPT_COST, CONST_CS | CONST_PERSISTENT); @@ -46,23 +46,24 @@ PHP_MINIT_FUNCTION(password) /* {{{ */ } /* }}} */ -static char* php_password_get_algo_name(const php_password_algos algo) +static char* php_password_get_algo_name(const php_password_algo algo) { switch (algo) { - case PASSWORD_BCRYPT: + case PHP_PASSWORD_BCRYPT: return "bcrypt"; + case PHP_PASSWORD_UNKNOWN: default: return "unknown"; } } -static php_password_algos php_password_determine_algo(const char *hash, const size_t len) +static php_password_algo php_password_determine_algo(const char *hash, const size_t len) { if (len > 3 && hash[0] == '$' && hash[1] == '2' && hash[2] == 'y' && len == 60) { - return PASSWORD_BCRYPT; + return PHP_PASSWORD_BCRYPT; } - return PASSWORD_UNKNOWN; + return PHP_PASSWORD_UNKNOWN; } static zend_bool php_password_salt_is_alphabet(const char *str, const size_t len) /* {{{ */ @@ -174,13 +175,13 @@ static int php_password_make_salt(size_t length, char *ret TSRMLS_DC) /* {{{ */ PHP_FUNCTION(password_get_info) { - php_password_algos algo; + php_password_algo algo; int hash_len; - char *hash, *algoName; + char *hash, *algo_name; zval *options; if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &hash, &hash_len) == FAILURE) { - RETURN_NULL(); + return; } if (hash_len < 0 || (size_t) hash_len < 0) { @@ -192,17 +193,17 @@ PHP_FUNCTION(password_get_info) array_init(options); algo = php_password_determine_algo(hash, (size_t) hash_len); - algoName = php_password_get_algo_name(algo); + algo_name = php_password_get_algo_name(algo); switch (algo) { - case PASSWORD_BCRYPT: + case PHP_PASSWORD_BCRYPT: { long cost = PHP_PASSWORD_BCRYPT_COST; sscanf(hash, "$2y$%ld$", &cost); add_assoc_long(options, "cost", cost); } break; - case PASSWORD_UNKNOWN: + case PHP_PASSWORD_UNKNOWN: default: break; } @@ -210,21 +211,21 @@ PHP_FUNCTION(password_get_info) array_init(return_value); add_assoc_long(return_value, "algo", algo); - add_assoc_string(return_value, "algoName", algoName, 1); + add_assoc_string(return_value, "algoName", algo_name, 1); add_assoc_zval(return_value, "options", options); } PHP_FUNCTION(password_needs_rehash) { long new_algo = 0; - php_password_algos algo; + php_password_algo algo; int hash_len; char *hash; HashTable *options = 0; zval **option_buffer; if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "sl|H", &hash, &hash_len, &new_algo, &options) == FAILURE) { - RETURN_NULL(); + return; } if (hash_len < 0) { @@ -239,7 +240,7 @@ PHP_FUNCTION(password_needs_rehash) } switch (algo) { - case PASSWORD_BCRYPT: + case PHP_PASSWORD_BCRYPT: { int newCost = PHP_PASSWORD_BCRYPT_COST, cost = 0; @@ -255,7 +256,7 @@ PHP_FUNCTION(password_needs_rehash) } } break; - case PASSWORD_UNKNOWN: + case PHP_PASSWORD_UNKNOWN: default: break; } @@ -309,11 +310,11 @@ PHP_FUNCTION(password_hash) zval **option_buffer; if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "sl|H", &password, &password_len, &algo, &options) == FAILURE) { - RETURN_NULL(); + return; } switch (algo) { - case PASSWORD_BCRYPT: + case PHP_PASSWORD_BCRYPT: { long cost = PHP_PASSWORD_BCRYPT_COST; @@ -334,6 +335,7 @@ PHP_FUNCTION(password_hash) hash_format_len = 7; } break; + case PHP_PASSWORD_UNKNOWN: default: php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unknown password hashing algorithm: %ld", algo); RETURN_NULL(); diff --git a/ext/standard/php_password.h b/ext/standard/php_password.h index c812e2c492ae5..079f187703879 100644 --- a/ext/standard/php_password.h +++ b/ext/standard/php_password.h @@ -28,14 +28,14 @@ PHP_FUNCTION(password_get_info); PHP_MINIT_FUNCTION(password); -#define PHP_PASSWORD_DEFAULT PASSWORD_BCRYPT +#define PHP_PASSWORD_DEFAULT PHP_PASSWORD_BCRYPT #define PHP_PASSWORD_BCRYPT_COST 10 typedef enum { - PASSWORD_UNKNOWN, - PASSWORD_BCRYPT -} php_password_algos; + PHP_PASSWORD_UNKNOWN, + PHP_PASSWORD_BCRYPT +} php_password_algo; #endif From 44c2624f8c7d6bc00f46bc69c77791c2a334cc9a Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Mon, 17 Sep 2012 10:59:51 -0400 Subject: [PATCH 36/44] Fix ucwords error casing --- ext/standard/password.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index 6c2a9af3aa3ae..8e9d8941b578c 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -185,7 +185,7 @@ PHP_FUNCTION(password_get_info) } if (hash_len < 0 || (size_t) hash_len < 0) { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Supplied Password Hash Too Long To Safely Identify"); + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Supplied password hash too long to safely identify"); RETURN_FALSE; } @@ -229,7 +229,7 @@ PHP_FUNCTION(password_needs_rehash) } if (hash_len < 0) { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Supplied Password Hash Too Long To Safely Identify"); + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Supplied password hash too long to safely identify"); RETURN_FALSE; } From 6fd5ba5c8d70ecbd80175a488160f57380d8afee Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Mon, 17 Sep 2012 11:10:59 -0400 Subject: [PATCH 37/44] Fix arg info for required params passed to needs_rehash --- ext/standard/basic_functions.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c index cf2266c31dd85..a30579e14352a 100644 --- a/ext/standard/basic_functions.c +++ b/ext/standard/basic_functions.c @@ -1863,7 +1863,7 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_INFO_EX(arginfo_password_get_info, 0, 0, 1) ZEND_ARG_INFO(0, hash) ZEND_END_ARG_INFO() -ZEND_BEGIN_ARG_INFO_EX(arginfo_password_needs_rehash, 0, 0, 1) +ZEND_BEGIN_ARG_INFO_EX(arginfo_password_needs_rehash, 0, 0, 2) ZEND_ARG_INFO(0, hash) ZEND_ARG_INFO(0, algo) ZEND_ARG_INFO(0, options) From 8bd79d180716fc521a3f5cae4bbfa96eb6397925 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Mon, 17 Sep 2012 11:43:47 -0400 Subject: [PATCH 38/44] Refactor slightly to enable cleaner readability --- ext/standard/password.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index 8e9d8941b578c..e8762690bb5b6 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -242,16 +242,16 @@ PHP_FUNCTION(password_needs_rehash) switch (algo) { case PHP_PASSWORD_BCRYPT: { - int newCost = PHP_PASSWORD_BCRYPT_COST, cost = 0; + long new_cost = PHP_PASSWORD_BCRYPT_COST, cost = 0; - if (options && zend_symtable_find(options, "cost", 5, (void **) &option_buffer) == SUCCESS) { + if (options && zend_symtable_find(options, "cost", sizeof("cost"), (void **) &option_buffer) == SUCCESS) { convert_to_long_ex(option_buffer); - newCost = Z_LVAL_PP(option_buffer); + new_cost = Z_LVAL_PP(option_buffer); zval_ptr_dtor(option_buffer); } - sscanf(hash, "$2y$%d$", &cost); - if (cost != newCost) { + sscanf(hash, "$2y$%ld$", &cost); + if (cost != new_cost) { RETURN_TRUE; } } From 4a7d18c79ef956022090cf7e8159ca6d50ae2339 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Fri, 5 Oct 2012 15:31:58 -0400 Subject: [PATCH 39/44] Fix some double free issues, and more cleanup work --- ext/standard/password.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index e8762690bb5b6..87fc2c2a227e1 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -79,7 +79,7 @@ static zend_bool php_password_salt_is_alphabet(const char *str, const size_t len } /* }}} */ -static int php_password_salt_to64(const char *str, const size_t str_len, const size_t out_len, char *ret) /* {{{ */ +static zend_bool php_password_salt_to64(const char *str, const size_t str_len, const size_t out_len, char *ret) /* {{{ */ { size_t pos = 0; size_t ret_len = 0; @@ -108,7 +108,7 @@ 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 TSRMLS_DC) /* {{{ */ +static zend_bool php_password_make_salt(size_t length, char *ret TSRMLS_DC) /* {{{ */ { int buffer_valid = 0; size_t i, raw_length; @@ -163,9 +163,8 @@ static int php_password_make_salt(size_t length, char *ret TSRMLS_DC) /* {{{ */ efree(buffer); efree(result); return FAILURE; - } else { - memcpy(ret, result, (int) length); } + memcpy(ret, result, (int) length); efree(result); efree(buffer); ret[length] = 0; @@ -245,9 +244,13 @@ PHP_FUNCTION(password_needs_rehash) long new_cost = PHP_PASSWORD_BCRYPT_COST, cost = 0; if (options && zend_symtable_find(options, "cost", sizeof("cost"), (void **) &option_buffer) == SUCCESS) { - convert_to_long_ex(option_buffer); - new_cost = Z_LVAL_PP(option_buffer); - zval_ptr_dtor(option_buffer); + if (Z_TYPE_PP(option_buffer) != IS_LONG) { + convert_to_long_ex(option_buffer); + new_cost = Z_LVAL_PP(option_buffer); + zval_ptr_dtor(option_buffer); + } else { + new_cost = Z_LVAL_PP(option_buffer); + } } sscanf(hash, "$2y$%ld$", &cost); @@ -319,9 +322,13 @@ PHP_FUNCTION(password_hash) long cost = PHP_PASSWORD_BCRYPT_COST; if (options && zend_symtable_find(options, "cost", 5, (void **) &option_buffer) == SUCCESS) { - convert_to_long_ex(option_buffer); - cost = Z_LVAL_PP(option_buffer); - zval_ptr_dtor(option_buffer); + if (Z_TYPE_PP(option_buffer) != IS_LONG) { + convert_to_long_ex(option_buffer); + cost = Z_LVAL_PP(option_buffer); + zval_ptr_dtor(option_buffer); + } else { + cost = Z_LVAL_PP(option_buffer); + } } if (cost < 4 || cost > 31) { @@ -367,14 +374,12 @@ PHP_FUNCTION(password_hash) case IS_RESOURCE: case IS_ARRAY: default: - zval_ptr_dtor(option_buffer); efree(hash_format); php_error_docref(NULL TSRMLS_CC, E_WARNING, "Non-string salt parameter supplied"); RETURN_NULL(); } if (buffer_len < required_salt_len) { efree(hash_format); - zval_ptr_dtor(option_buffer); php_error_docref(NULL TSRMLS_CC, E_WARNING, "Provided salt is too short: %lu expecting %lu", (unsigned long) buffer_len, (unsigned long) required_salt_len); RETURN_NULL(); } else if (0 == php_password_salt_is_alphabet(buffer, buffer_len)) { @@ -382,7 +387,6 @@ PHP_FUNCTION(password_hash) if (php_password_salt_to64(buffer, buffer_len, required_salt_len, salt) == FAILURE) { efree(hash_format); efree(salt); - zval_ptr_dtor(option_buffer); php_error_docref(NULL TSRMLS_CC, E_WARNING, "Provided salt is too short: %lu", (unsigned long) buffer_len); RETURN_NULL(); } @@ -392,7 +396,6 @@ PHP_FUNCTION(password_hash) memcpy(salt, buffer, (int) required_salt_len); salt_len = required_salt_len; } - zval_ptr_dtor(option_buffer); } else { salt = safe_emalloc(required_salt_len, 1, 1); if (php_password_make_salt(required_salt_len, salt TSRMLS_CC) == FAILURE) { From 25b2d364e995fc070ae16ee34f60d25148413769 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Fri, 5 Oct 2012 15:53:40 -0400 Subject: [PATCH 40/44] Fix issue with possible memory leak --- ext/standard/password.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index 87fc2c2a227e1..af42a6f5b9671 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -350,7 +350,7 @@ PHP_FUNCTION(password_hash) if (options && zend_symtable_find(options, "salt", 5, (void**) &option_buffer) == SUCCESS) { char *buffer; - int buffer_len_int; + int buffer_len_int = 0; size_t buffer_len; switch (Z_TYPE_PP(option_buffer)) { case IS_NULL: @@ -359,17 +359,20 @@ PHP_FUNCTION(password_hash) case IS_DOUBLE: case IS_BOOL: case IS_OBJECT: - convert_to_string_ex(option_buffer); if (Z_TYPE_PP(option_buffer) == IS_STRING) { buffer = Z_STRVAL_PP(option_buffer); buffer_len_int = Z_STRLEN_PP(option_buffer); - if (buffer_len_int < 0) { + break; + } else { + SEPARATE_ZVAL(option_buffer); + convert_to_string_ex(option_buffer); + if (Z_TYPE_PP(option_buffer) == IS_STRING) { + buffer = Z_STRVAL_PP(option_buffer); + buffer_len_int = Z_STRLEN_PP(option_buffer); zval_ptr_dtor(option_buffer); - efree(hash_format); - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Supplied salt is too long"); + break; } - buffer_len = (size_t) buffer_len_int; - break; + zval_ptr_dtor(option_buffer); } case IS_RESOURCE: case IS_ARRAY: @@ -378,6 +381,11 @@ PHP_FUNCTION(password_hash) php_error_docref(NULL TSRMLS_CC, E_WARNING, "Non-string salt parameter supplied"); RETURN_NULL(); } + if (buffer_len_int < 0) { + efree(hash_format); + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Supplied salt is too long"); + } + buffer_len = (size_t) buffer_len_int; if (buffer_len < required_salt_len) { efree(hash_format); php_error_docref(NULL TSRMLS_CC, E_WARNING, "Provided salt is too short: %lu expecting %lu", (unsigned long) buffer_len, (unsigned long) required_salt_len); From 1751d5fabeff466f08da560caa6f92222ade5a82 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Sat, 6 Oct 2012 10:38:41 -0400 Subject: [PATCH 41/44] Really fix leaks, add test cases to prove it... --- ext/standard/password.c | 54 +++++++++++-------- .../password/password_bcrypt_errors.phpt | 11 ++++ .../tests/password/password_hash_error.phpt | 5 ++ .../tests/password/password_needs_rehash.phpt | 6 +++ 4 files changed, 54 insertions(+), 22 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index af42a6f5b9671..9667fdcade375 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -245,9 +245,12 @@ PHP_FUNCTION(password_needs_rehash) if (options && zend_symtable_find(options, "cost", sizeof("cost"), (void **) &option_buffer) == SUCCESS) { if (Z_TYPE_PP(option_buffer) != IS_LONG) { - convert_to_long_ex(option_buffer); - new_cost = Z_LVAL_PP(option_buffer); - zval_ptr_dtor(option_buffer); + zval *cast_option_buffer; + ALLOC_ZVAL(cast_option_buffer); + INIT_PZVAL_COPY(cast_option_buffer, *option_buffer); + convert_to_long(cast_option_buffer); + new_cost = Z_LVAL_P(cast_option_buffer); + zval_dtor(cast_option_buffer); } else { new_cost = Z_LVAL_PP(option_buffer); } @@ -323,9 +326,12 @@ PHP_FUNCTION(password_hash) if (options && zend_symtable_find(options, "cost", 5, (void **) &option_buffer) == SUCCESS) { if (Z_TYPE_PP(option_buffer) != IS_LONG) { - convert_to_long_ex(option_buffer); - cost = Z_LVAL_PP(option_buffer); - zval_ptr_dtor(option_buffer); + zval *cast_option_buffer; + ALLOC_ZVAL(cast_option_buffer); + INIT_PZVAL_COPY(cast_option_buffer, *option_buffer); + convert_to_long(cast_option_buffer); + cost = Z_LVAL_P(cast_option_buffer); + zval_dtor(cast_option_buffer); } else { cost = Z_LVAL_PP(option_buffer); } @@ -353,27 +359,27 @@ PHP_FUNCTION(password_hash) int buffer_len_int = 0; size_t buffer_len; switch (Z_TYPE_PP(option_buffer)) { - case IS_NULL: case IS_STRING: + buffer = estrndup(Z_STRVAL_PP(option_buffer), Z_STRLEN_PP(option_buffer)); + buffer_len_int = Z_STRLEN_PP(option_buffer); + break; case IS_LONG: case IS_DOUBLE: - case IS_BOOL: - case IS_OBJECT: - if (Z_TYPE_PP(option_buffer) == IS_STRING) { - buffer = Z_STRVAL_PP(option_buffer); - buffer_len_int = Z_STRLEN_PP(option_buffer); + case IS_OBJECT: { + zval *cast_option_buffer; + ALLOC_ZVAL(cast_option_buffer); + INIT_PZVAL_COPY(cast_option_buffer, *option_buffer); + convert_to_string(cast_option_buffer); + if (Z_TYPE_P(cast_option_buffer) == IS_STRING) { + buffer = estrndup(Z_STRVAL_P(cast_option_buffer), Z_STRLEN_P(cast_option_buffer)); + buffer_len_int = Z_STRLEN_P(cast_option_buffer); + zval_dtor(cast_option_buffer); break; - } else { - SEPARATE_ZVAL(option_buffer); - convert_to_string_ex(option_buffer); - if (Z_TYPE_PP(option_buffer) == IS_STRING) { - buffer = Z_STRVAL_PP(option_buffer); - buffer_len_int = Z_STRLEN_PP(option_buffer); - zval_ptr_dtor(option_buffer); - break; - } - zval_ptr_dtor(option_buffer); } + zval_dtor(cast_option_buffer); + } + case IS_BOOL: + case IS_NULL: case IS_RESOURCE: case IS_ARRAY: default: @@ -383,17 +389,20 @@ PHP_FUNCTION(password_hash) } if (buffer_len_int < 0) { efree(hash_format); + efree(buffer); php_error_docref(NULL TSRMLS_CC, E_WARNING, "Supplied salt is too long"); } buffer_len = (size_t) buffer_len_int; if (buffer_len < required_salt_len) { efree(hash_format); + efree(buffer); php_error_docref(NULL TSRMLS_CC, E_WARNING, "Provided salt is too short: %lu expecting %lu", (unsigned long) buffer_len, (unsigned long) required_salt_len); RETURN_NULL(); } else if (0 == php_password_salt_is_alphabet(buffer, buffer_len)) { salt = safe_emalloc(required_salt_len, 1, 1); if (php_password_salt_to64(buffer, buffer_len, required_salt_len, salt) == FAILURE) { efree(hash_format); + efree(buffer); efree(salt); php_error_docref(NULL TSRMLS_CC, E_WARNING, "Provided salt is too short: %lu", (unsigned long) buffer_len); RETURN_NULL(); @@ -404,6 +413,7 @@ PHP_FUNCTION(password_hash) memcpy(salt, buffer, (int) required_salt_len); salt_len = required_salt_len; } + efree(buffer); } else { salt = safe_emalloc(required_salt_len, 1, 1); if (php_password_make_salt(required_salt_len, salt TSRMLS_CC) == FAILURE) { diff --git a/ext/standard/tests/password/password_bcrypt_errors.phpt b/ext/standard/tests/password/password_bcrypt_errors.phpt index f36d11f694185..2548c9accb805 100644 --- a/ext/standard/tests/password/password_bcrypt_errors.phpt +++ b/ext/standard/tests/password/password_bcrypt_errors.phpt @@ -12,6 +12,10 @@ var_dump(password_hash("foo", PASSWORD_BCRYPT, array("salt" => "foo"))); var_dump(password_hash("foo", PASSWORD_BCRYPT, array("salt" => "123456789012345678901"))); +var_dump(password_hash("foo", PASSWORD_BCRYPT, array("salt" => 123))); + +var_dump(password_hash("foo", PASSWORD_BCRYPT, array("cost" => "foo"))); + ?> --EXPECTF-- Warning: password_hash(): Invalid bcrypt cost parameter specified: 3 in %s on line %d @@ -26,3 +30,10 @@ NULL Warning: password_hash(): Provided salt is too short: 21 expecting 22 in %s on line %d NULL +Warning: password_hash(): Provided salt is too short: 3 expecting 22 in %s on line %d +NULL + +Warning: password_hash(): Invalid bcrypt cost parameter specified: 0 in %s on line %d +NULL + + diff --git a/ext/standard/tests/password/password_hash_error.phpt b/ext/standard/tests/password/password_hash_error.phpt index 695a6c479ad51..952250cb309b0 100644 --- a/ext/standard/tests/password/password_hash_error.phpt +++ b/ext/standard/tests/password/password_hash_error.phpt @@ -18,6 +18,9 @@ var_dump(password_hash(array(), PASSWORD_BCRYPT)); var_dump(password_hash("123", PASSWORD_BCRYPT, array("salt" => array()))); +/* Non-string salt, checking for memory leaks */ +var_dump(password_hash('123', PASSWORD_BCRYPT, array('salt' => 1234))); + ?> --EXPECTF-- Warning: password_hash() expects at least 2 parameters, 0 given in %s on line %d @@ -41,3 +44,5 @@ NULL Warning: password_hash(): Non-string salt parameter supplied in %s on line %d NULL +Warning: password_hash(): Provided salt is too short: 4 expecting 22 in %s on line %d +NULL diff --git a/ext/standard/tests/password/password_needs_rehash.phpt b/ext/standard/tests/password/password_needs_rehash.phpt index 2fc3983980165..734729e63d705 100644 --- a/ext/standard/tests/password/password_needs_rehash.phpt +++ b/ext/standard/tests/password/password_needs_rehash.phpt @@ -26,6 +26,11 @@ var_dump(password_needs_rehash('$2y$10$MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR.oP7XOR9H $cost = str_pad(PASSWORD_BCRYPT_DEFAULT_COST, 2, '0', STR_PAD_LEFT); var_dump(password_needs_rehash('$2y$'.$cost.'$MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR.oP7XOR9HD/vjUuOj100y', PASSWORD_BCRYPT)); +// Should Issue Needs Rehash, Since Foo is cast to 0... +var_dump(password_needs_rehash('$2y$10$MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR.oP7XOR9HD/vjUuOj100y', PASSWORD_BCRYPT, array('cost' => 'foo'))); + + + echo "OK!"; ?> --EXPECT-- @@ -36,4 +41,5 @@ bool(false) bool(true) bool(true) bool(false) +bool(true) OK! From 76e83f769ff5929b45cf0ac666335ce68ada166f Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Sat, 6 Oct 2012 12:33:48 -0400 Subject: [PATCH 42/44] fix allocation and copy issue --- ext/standard/password.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index 9667fdcade375..70004a9bc8898 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -247,7 +247,7 @@ PHP_FUNCTION(password_needs_rehash) if (Z_TYPE_PP(option_buffer) != IS_LONG) { zval *cast_option_buffer; ALLOC_ZVAL(cast_option_buffer); - INIT_PZVAL_COPY(cast_option_buffer, *option_buffer); + MAKE_COPY_ZVAL(option_buffer, cast_option_buffer); convert_to_long(cast_option_buffer); new_cost = Z_LVAL_P(cast_option_buffer); zval_dtor(cast_option_buffer); @@ -328,7 +328,7 @@ PHP_FUNCTION(password_hash) if (Z_TYPE_PP(option_buffer) != IS_LONG) { zval *cast_option_buffer; ALLOC_ZVAL(cast_option_buffer); - INIT_PZVAL_COPY(cast_option_buffer, *option_buffer); + MAKE_COPY_ZVAL(option_buffer, cast_option_buffer); convert_to_long(cast_option_buffer); cost = Z_LVAL_P(cast_option_buffer); zval_dtor(cast_option_buffer); @@ -368,7 +368,7 @@ PHP_FUNCTION(password_hash) case IS_OBJECT: { zval *cast_option_buffer; ALLOC_ZVAL(cast_option_buffer); - INIT_PZVAL_COPY(cast_option_buffer, *option_buffer); + MAKE_COPY_ZVAL(option_buffer, cast_option_buffer); convert_to_string(cast_option_buffer); if (Z_TYPE_P(cast_option_buffer) == IS_STRING) { buffer = estrndup(Z_STRVAL_P(cast_option_buffer), Z_STRLEN_P(cast_option_buffer)); From 37b2207f66ac1cebdc3ff3f7f88ec319ee893292 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Sun, 7 Oct 2012 05:12:02 -0400 Subject: [PATCH 43/44] Clean up unreported memory leak by switching to zval_ptr_dtor --- ext/standard/password.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index 70004a9bc8898..3507183c2a58e 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -250,7 +250,7 @@ PHP_FUNCTION(password_needs_rehash) MAKE_COPY_ZVAL(option_buffer, cast_option_buffer); convert_to_long(cast_option_buffer); new_cost = Z_LVAL_P(cast_option_buffer); - zval_dtor(cast_option_buffer); + zval_ptr_dtor(&cast_option_buffer); } else { new_cost = Z_LVAL_PP(option_buffer); } @@ -331,7 +331,7 @@ PHP_FUNCTION(password_hash) MAKE_COPY_ZVAL(option_buffer, cast_option_buffer); convert_to_long(cast_option_buffer); cost = Z_LVAL_P(cast_option_buffer); - zval_dtor(cast_option_buffer); + zval_ptr_dtor(&cast_option_buffer); } else { cost = Z_LVAL_PP(option_buffer); } @@ -373,10 +373,10 @@ PHP_FUNCTION(password_hash) if (Z_TYPE_P(cast_option_buffer) == IS_STRING) { buffer = estrndup(Z_STRVAL_P(cast_option_buffer), Z_STRLEN_P(cast_option_buffer)); buffer_len_int = Z_STRLEN_P(cast_option_buffer); - zval_dtor(cast_option_buffer); + zval_ptr_dtor(&cast_option_buffer); break; } - zval_dtor(cast_option_buffer); + zval_ptr_dtor(&cast_option_buffer); } case IS_BOOL: case IS_NULL: From 0bc9ca39ced4128c3b9fb1ba2ac797d342e7eef2 Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Sun, 7 Oct 2012 05:42:08 -0400 Subject: [PATCH 44/44] Refactor to using a stack based zval instead of dynamic allocation --- ext/standard/password.c | 39 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index 3507183c2a58e..266ad0a42174a 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -245,12 +245,11 @@ PHP_FUNCTION(password_needs_rehash) if (options && zend_symtable_find(options, "cost", sizeof("cost"), (void **) &option_buffer) == SUCCESS) { if (Z_TYPE_PP(option_buffer) != IS_LONG) { - zval *cast_option_buffer; - ALLOC_ZVAL(cast_option_buffer); - MAKE_COPY_ZVAL(option_buffer, cast_option_buffer); - convert_to_long(cast_option_buffer); - new_cost = Z_LVAL_P(cast_option_buffer); - zval_ptr_dtor(&cast_option_buffer); + zval cast_option_buffer; + MAKE_COPY_ZVAL(option_buffer, &cast_option_buffer); + convert_to_long(&cast_option_buffer); + new_cost = Z_LVAL(cast_option_buffer); + zval_dtor(&cast_option_buffer); } else { new_cost = Z_LVAL_PP(option_buffer); } @@ -326,12 +325,11 @@ PHP_FUNCTION(password_hash) if (options && zend_symtable_find(options, "cost", 5, (void **) &option_buffer) == SUCCESS) { if (Z_TYPE_PP(option_buffer) != IS_LONG) { - zval *cast_option_buffer; - ALLOC_ZVAL(cast_option_buffer); - MAKE_COPY_ZVAL(option_buffer, cast_option_buffer); - convert_to_long(cast_option_buffer); - cost = Z_LVAL_P(cast_option_buffer); - zval_ptr_dtor(&cast_option_buffer); + zval cast_option_buffer; + MAKE_COPY_ZVAL(option_buffer, &cast_option_buffer); + convert_to_long(&cast_option_buffer); + cost = Z_LVAL(cast_option_buffer); + zval_dtor(&cast_option_buffer); } else { cost = Z_LVAL_PP(option_buffer); } @@ -366,17 +364,16 @@ PHP_FUNCTION(password_hash) case IS_LONG: case IS_DOUBLE: case IS_OBJECT: { - zval *cast_option_buffer; - ALLOC_ZVAL(cast_option_buffer); - MAKE_COPY_ZVAL(option_buffer, cast_option_buffer); - convert_to_string(cast_option_buffer); - if (Z_TYPE_P(cast_option_buffer) == IS_STRING) { - buffer = estrndup(Z_STRVAL_P(cast_option_buffer), Z_STRLEN_P(cast_option_buffer)); - buffer_len_int = Z_STRLEN_P(cast_option_buffer); - zval_ptr_dtor(&cast_option_buffer); + zval cast_option_buffer; + MAKE_COPY_ZVAL(option_buffer, &cast_option_buffer); + convert_to_string(&cast_option_buffer); + if (Z_TYPE(cast_option_buffer) == IS_STRING) { + buffer = estrndup(Z_STRVAL(cast_option_buffer), Z_STRLEN(cast_option_buffer)); + buffer_len_int = Z_STRLEN(cast_option_buffer); + zval_dtor(&cast_option_buffer); break; } - zval_ptr_dtor(&cast_option_buffer); + zval_dtor(&cast_option_buffer); } case IS_BOOL: case IS_NULL: