From c77f2c29585f97bd9dad533b9d2bc8334de34f1b Mon Sep 17 00:00:00 2001 From: Anthony Ferrara Date: Sun, 24 Jun 2012 22:44:43 -0400 Subject: [PATCH 01/48] 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 63d40efde44..64025db6257 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 c33ae1e05c8..fba423b1919 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 d14b859e9db..5f24641b4d3 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 00000000000..677f1325caa --- /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 00000000000..f813189a46b --- /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 483dbc33bc8..bccfebe5433 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/48] 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 677f1325caa..9201ff3f8d1 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 f813189a46b..5967840d51d 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/48] 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 9201ff3f8d1..665e69f28ce 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/48] 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 665e69f28ce..2b7e7dfc937 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/48] 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 2b7e7dfc937..f6d8048dac8 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/48] 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 f6d8048dac8..013dab7c6c5 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/48] 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 013dab7c6c5..f2c94fb473b 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/48] Refactor salt generation, rename password_create to password_hash --- ext/standard/basic_functions.c | 4 +- ext/standard/password.c | 101 +++++++++++++-------------------- ext/standard/php_password.h | 6 +- 3 files changed, 43 insertions(+), 68 deletions(-) diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c index 64025db6257..9e35a5e020b 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 f2c94fb473b..f049fbcbf1e 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); - 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 PHP_WIN32 + { + BYTE *iv_b = (BYTE *) buffer; + if (php_win32_get_random_bytes(iv_b, (size_t) raw_length) == SUCCESS) { + buffer_valid = 1; + } + } +#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; + } + } +#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 5967840d51d..830d31ce64b 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/48] 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 9e35a5e020b..5dc86ab0978 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 f049fbcbf1e..94aa4dc3e3e 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 830d31ce64b..81fe41f529f 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 cc04b1317e9..e52c32c57dc 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 a5a7a4a81f8..5f1205e6a1d 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 5d8f26e0fd3..927f305cde8 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/48] 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 00000000000..42238173501 --- /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 00000000000..ecefa10af3a --- /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 00000000000..dfbb094b39d --- /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 00000000000..63b56f85446 --- /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 00000000000..7d79713e8dd --- /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 00000000000..e7ecc7edd30 --- /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 00000000000..3e653fa04e4 --- /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/48] 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 ecefa10af3a..2fca8b71bc3 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/48] 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 94aa4dc3e3e..ab115afb6c7 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 7d79713e8dd..8078582e3c8 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/48] Fix formatting issues in password.c --- ext/standard/password.c | 137 +++++++++++++++++++++------------------- 1 file changed, 71 insertions(+), 66 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index ab115afb6c7..e0e260a0c19 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; + 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; + 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/48] Refactor crypt to use an external working function --- ext/standard/crypt.c | 256 +++++++++++++++++++++------------------ ext/standard/php_crypt.h | 1 + 2 files changed, 137 insertions(+), 120 deletions(-) diff --git a/ext/standard/crypt.c b/ext/standard/crypt.c index 9a1fcf1f694..a592a4b37c0 100644 --- a/ext/standard/crypt.c +++ b/ext/standard/crypt.c @@ -145,14 +145,141 @@ 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) +{ + char *crypt_res; +/* Windows (win32/crypt) has a stripped down version of libxcrypt and + a CryptoApi md5_crypt implementation */ +#if PHP_USE_PHP_CRYPT_R + { + struct php_crypt_extended_data buffer; + + if (salt[0]=='$' && salt[1]=='1' && salt[2]=='$') { + 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 + + PHP_MAX_SALT_LEN + 43 + 1); + output = emalloc(needed); + + crypt_res = php_sha512_crypt_r(password, salt, output, needed); + if (!crypt_res) { + memset(output, 0, needed); + efree(output); + return FAILURE; + } else { + *result = 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 + + PHP_MAX_SALT_LEN + 43 + 1); + output = emalloc(needed); + + crypt_res = php_sha256_crypt_r(password, salt, output, needed); + if (!crypt_res) { + memset(output, 0, needed); + efree(output); + return FAILURE; + } else { + *result = output; + return SUCCESS; + } + } else if ( + salt[0] == '$' && + salt[1] == '2' && + salt[2] >= 'a' && salt[2] <= 'z' && + salt[3] == '$' && + salt[4] >= '0' && salt[4] <= '3' && + salt[5] >= '0' && salt[5] <= '9' && + salt[6] == '$') { + char output[PHP_MAX_SALT_LEN + 1]; + + memset(output, 0, PHP_MAX_SALT_LEN + 1); + + crypt_res = php_crypt_blowfish_rn(password, salt, output, sizeof(output)); + if (!crypt_res) { + 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; + memset(output, 0, PHP_MAX_SALT_LEN + 1); + return SUCCESS; + } + } else { + memset(&buffer, 0, sizeof(buffer)); + _crypt_extended_init_r(); + + crypt_res = _crypt_extended_r(password, salt, &buffer); + 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; + return SUCCESS; + } + } + } +#else + +# if defined(HAVE_CRYPT_R) && (defined(_REENTRANT) || defined(_THREAD_SAFE)) + { +# if defined(CRYPT_R_STRUCT_CRYPT_DATA) + struct crypt_data buffer; + memset(&buffer, 0, sizeof(buffer)); +# elif defined(CRYPT_R_CRYPTD) + CRYPTD buffer; +# else +# error Data struct used by crypt_r() is unknown. Please report. +# endif + crypt_res = crypt_r(password, salt, &buffer); + 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'; + 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; + char *str, *salt_in = NULL, *result = 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 @@ -182,128 +309,17 @@ PHP_FUNCTION(crypt) } else { salt_in_len = MIN(PHP_MAX_SALT_LEN, salt_in_len); } + salt[salt_in_len] = '\0'; -/* Windows (win32/crypt) has a stripped down version of libxcrypt and - a CryptoApi md5_crypt implementation */ -#if PHP_USE_PHP_CRYPT_R - { - 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); - } 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); - output = emalloc(needed); - salt[salt_in_len] = '\0'; - - crypt_res = php_sha512_crypt_r(str, salt, output, needed); - if (!crypt_res) { - if (salt[0]=='*' && salt[1]=='0') { - RETVAL_STRING("*1", 1); - } else { - RETVAL_STRING("*0", 1); - } - } else { - RETVAL_STRING(output, 1); - } - - 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); - output = emalloc(needed); - salt[salt_in_len] = '\0'; - - crypt_res = php_sha256_crypt_r(str, salt, output, needed); - if (!crypt_res) { - if (salt[0]=='*' && salt[1]=='0') { - RETVAL_STRING("*1", 1); - } else { - RETVAL_STRING("*0", 1); - } - } else { - RETVAL_STRING(output, 1); - } - - memset(output, 0, PHP_MAX_SALT_LEN + 1); - efree(output); - } else if ( - salt[0] == '$' && - salt[1] == '2' && - salt[2] >= 'a' && salt[2] <= 'z' && - salt[3] == '$' && - salt[4] >= '0' && salt[4] <= '3' && - salt[5] >= '0' && salt[5] <= '9' && - salt[6] == '$') { - char output[PHP_MAX_SALT_LEN + 1]; - - memset(output, 0, PHP_MAX_SALT_LEN + 1); - - crypt_res = php_crypt_blowfish_rn(str, salt, output, sizeof(output)); - if (!crypt_res) { - if (salt[0]=='*' && salt[1]=='0') { - RETVAL_STRING("*1", 1); - } else { - RETVAL_STRING("*0", 1); - } - } else { - RETVAL_STRING(output, 1); - } - - memset(output, 0, PHP_MAX_SALT_LEN + 1); + if (crypt_execute(str, str_len, salt, salt_in_len, &result) == FAILURE) { + if (salt[0] == '*' && salt[1] == '0') { + RETURN_STRING("*1", 1); } else { - memset(&buffer, 0, sizeof(buffer)); - _crypt_extended_init_r(); - - crypt_res = _crypt_extended_r(str, salt, &buffer); - if (!crypt_res) { - if (salt[0]=='*' && salt[1]=='0') { - RETURN_STRING("*1", 1); - } else { - RETURN_STRING("*0", 1); - } - } else { - RETURN_STRING(crypt_res, 1); - } + RETURN_STRING("*0", 1); } } -#else - -# if defined(HAVE_CRYPT_R) && (defined(_REENTRANT) || defined(_THREAD_SAFE)) - { -# if defined(CRYPT_R_STRUCT_CRYPT_DATA) - struct crypt_data buffer; - memset(&buffer, 0, sizeof(buffer)); -# elif defined(CRYPT_R_CRYPTD) - CRYPTD buffer; -# else -# error Data struct used by crypt_r() is unknown. Please report. -# endif - crypt_res = crypt_r(str, salt, &buffer); - if (!crypt_res) { - if (salt[0]=='*' && salt[1]=='0') { - RETURN_STRING("*1", 1); - } else { - RETURN_STRING("*0", 1); - } - } else { - RETURN_STRING(crypt_res, 1); - } - } -# endif -#endif + RETVAL_STRING(result, 1); + efree(result); } /* }}} */ #endif diff --git a/ext/standard/php_crypt.h b/ext/standard/php_crypt.h index 93b232896af..1dffb0bc3ed 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/48] Refactor password.c a bit, add different error checking --- ext/standard/password.c | 121 +++++++++--------- .../password/password_bcrypt_errors.phpt | 8 +- .../tests/password/password_hash_error.phpt | 18 +-- .../password/password_make_salt_error.phpt | 10 +- 4 files changed, 75 insertions(+), 82 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index e0e260a0c19..dfe624dcefa 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) { - RETURN_FALSE; - } - - zend_call_method_with_2_params(NULL, NULL, NULL, "crypt", &ret, password, hash); + int password_len, hash_len; + char *ret, *password, *hash; - if (Z_TYPE_P(ret) != IS_STRING) { - zval_ptr_dtor(&ret); + 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) { 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; + zval **option_buffer; - 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; - } - - 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); - - 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); + if (crypt_execute(password, password_len, hash, hash_format_len + salt_len, &result) == FAILURE) { + efree(hash); RETURN_FALSE; } - RETURN_ZVAL(ret, 0, 1); + efree(hash); + + if (strlen(result) < 13) { + efree(result); + RETURN_FALSE; + } + + 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 42238173501..f36d11f6941 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 dfbb094b39d..b82e23edc03 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 8078582e3c8..4a1d5e29c98 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/48] 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 25f5ec0107a..3b443fc4d5a 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 dfe624dcefa..982ae7d5ac3 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 1dffb0bc3ed..7410a8c3280 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/48] 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 982ae7d5ac3..558cf24c195 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/48] 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 558cf24c195..9c031524262 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 81fe41f529f..338665ea2f9 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 2fca8b71bc3..3b6fc0932ca 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 5f1205e6a1d..a5a7a4a81f8 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 927f305cde8..5d8f26e0fd3 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/48] 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 5dc86ab0978..9e35a5e020b 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 9c031524262..6de812057f2 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 338665ea2f9..57c6b887858 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 3b6fc0932ca..ff48b29b169 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 parameter 2 to be string, array given in %s on line %d +Warning: password_hash() expects at least 2 parameters, 1 given in %s on line %d NULL -Warning: password_hash(): Unknown password hashing algorithm: bar 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: 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/48] 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 6de812057f2..eb4abd2722f 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/48] 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 9e35a5e020b..bf6f9b0b583 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 eb4abd2722f..9bfb0235843 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 57c6b887858..45e68499368 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/48] 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 9bfb0235843..6da656c5afe 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/48] 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 bf6f9b0b583..e6500dd66bd 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 6da656c5afe..9be6f8c366f 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 45e68499368..90e4d89bc48 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/48] 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 9be6f8c366f..2f1ebb5c59d 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/48] 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 2f1ebb5c59d..2e5d62aceca 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 90e4d89bc48..8211ae17533 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 63b56f85446..c7aa51455ed 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 4a1d5e29c98..92df53ad0fe 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/48] 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 e6b155979fb..1f1b3d366db 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 2e5d62aceca..4f8ef5dcab7 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 8211ae17533..d99c061c008 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 c7aa51455ed..00000000000 --- 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 92df53ad0fe..00000000000 --- 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/48] 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 4f8ef5dcab7..d3dc4574286 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 d99c061c008..db7747a3db6 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/48] 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 00000000000..4c8dc04ff80 --- /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 00000000000..af676744c8b --- /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 00000000000..0c03d88b4d4 --- /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 00000000000..e25ef8db3f4 --- /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/48] 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 ff48b29b169..f59d3d5e488 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/48] 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 2f40dc91b7d..5eb9947fe7a 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/48] 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 d3dc4574286..9b1bb8cccaf 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 0c03d88b4d4..2fc39839801 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/48] Add news entry for password API --- NEWS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS b/NEWS index 1ee977974a5..08045fc23a1 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/48] 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 ece64f375f4..cf2266c31dd 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/48] 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 9b1bb8cccaf..0dd8fed6455 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 db7747a3db6..c812e2c492a 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/48] 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 0dd8fed6455..6c2a9af3aa3 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 c812e2c492a..079f1877038 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/48] 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 6c2a9af3aa3..8e9d8941b57 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/48] 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 cf2266c31dd..a30579e1435 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/48] 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 8e9d8941b57..e8762690bb5 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/48] 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 e8762690bb5..87fc2c2a227 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/48] Fix issue with possible memory leak --- ext/standard/password.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index 87fc2c2a227..af42a6f5b96 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) { - 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; + } 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); } 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/48] 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 af42a6f5b96..9667fdcade3 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 f36d11f6941..2548c9accb8 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 695a6c479ad..952250cb309 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 2fc39839801..734729e63d7 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/48] 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 9667fdcade3..70004a9bc88 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/48] 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 70004a9bc88..3507183c2a5 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/48] 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 3507183c2a5..266ad0a4217 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: From 7eba512b5170fc57dc3d4a6b93f98a0e0acc7721 Mon Sep 17 00:00:00 2001 From: Anatoliy Belsky Date: Tue, 16 Oct 2012 11:14:43 +0200 Subject: [PATCH 45/48] updated NEWS --- NEWS | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NEWS b/NEWS index 2ad1fa7920e..c2e3444cc52 100644 --- a/NEWS +++ b/NEWS @@ -86,4 +86,8 @@ PHP NEWS - Zip: . Upgraded libzip to 0.10.1 (Anatoliy) +- Fileinfo: + . Fixed bug #63248 (Load multiple magic files from a directory under Windows). + (Anatoliy) + <<< NOTE: Insert NEWS from last stable release here prior to actual release! >>> From 917639d4631b456f8ffd959a3c523071c3e9c8b5 Mon Sep 17 00:00:00 2001 From: ULF WENDEL Date: Sat, 29 Sep 2012 17:42:00 +0200 Subject: [PATCH 46/48] Updating expected output in anticipation of mysqlnd_auth.c path --- .../tests/mysqli_pam_sha256_public_key_option_invalid.phpt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ext/mysqli/tests/mysqli_pam_sha256_public_key_option_invalid.phpt b/ext/mysqli/tests/mysqli_pam_sha256_public_key_option_invalid.phpt index 960f08ad481..e2626240d82 100644 --- a/ext/mysqli/tests/mysqli_pam_sha256_public_key_option_invalid.phpt +++ b/ext/mysqli/tests/mysqli_pam_sha256_public_key_option_invalid.phpt @@ -182,5 +182,7 @@ Warning: mysqli::real_connect(): (HY000/1045): %s in %s on line %d [300 + 002] [1045] %s Warning: mysqli::real_connect(%sest_sha256_wrong_%d): failed to open stream: No such file or directory in %s on line %d + +Warning: mysqli::real_connect(): (HY000/1045): %s in %s on line %d [400 + 002] [1045] %s done! \ No newline at end of file From da541ff561e0ac6ac72d2efd8b785ecfeef868dc Mon Sep 17 00:00:00 2001 From: ULF WENDEL Date: Sat, 29 Sep 2012 18:54:54 +0200 Subject: [PATCH 47/48] Cover have_ssl=NO and have_ssl=DISABLED --- ext/mysqli/tests/bug51647.phpt | 4 ++-- ext/mysqli/tests/bug55283.phpt | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ext/mysqli/tests/bug51647.phpt b/ext/mysqli/tests/bug51647.phpt index b1c1e87a772..78540f1c33a 100644 --- a/ext/mysqli/tests/bug51647.phpt +++ b/ext/mysqli/tests/bug51647.phpt @@ -24,12 +24,12 @@ if ($res = $link->query('SHOW VARIABLES LIKE "have_ssl"')) { die(sprintf("skip Failed to test for MySQL SSL support, [%d] %s", $link->errno, $link->error)); } } - + if (empty($row)) die(sprintf("skip Failed to test for MySQL SSL support, [%d] %s", $link->errno, $link->error)); -if ($row[1] == 'NO') +if (($row[1] == 'NO') || ($row[1] == 'DISABLED')) die(sprintf("skip MySQL has no SSL support, [%d] %s", $link->errno, $link->error)); $link->close(); diff --git a/ext/mysqli/tests/bug55283.phpt b/ext/mysqli/tests/bug55283.phpt index 6000fce0a9b..d03daaee88e 100644 --- a/ext/mysqli/tests/bug55283.phpt +++ b/ext/mysqli/tests/bug55283.phpt @@ -29,7 +29,7 @@ if ($res = $link->query('SHOW VARIABLES LIKE "have_ssl"')) { if (empty($row)) die(sprintf("skip Failed to test for MySQL SSL support, [%d] %s", $link->errno, $link->error)); -if ($row[1] == 'NO') +if (($row[1] == 'NO') || ($row[1] == 'DISABLED')) die(sprintf("skip MySQL has no SSL support, [%d] %s", $link->errno, $link->error)); $link->close(); @@ -41,7 +41,7 @@ $link->close(); $flags = MYSQLI_CLIENT_SSL; - + $link = mysqli_init(); mysqli_ssl_set($link, null, null, null, null, "RC4-MD5"); if (my_mysqli_real_connect($link, 'p:' . $host, $user, $passwd, $db, $port, null, $flags)) { From 6d019deee206dd76396bcaff9497ae3619d279b0 Mon Sep 17 00:00:00 2001 From: Anatoliy Belsky Date: Tue, 16 Oct 2012 11:03:32 +0200 Subject: [PATCH 48/48] Fixed bug #63248 Load multiple magic files on win - adapt config.w32 to not to use dirent lib anymore - prevent libmagic from opening a dir handle under win - reimplement the dir iteration functionality with streams --- NEWS | 4 + ext/fileinfo/config.w32 | 26 ++--- ext/fileinfo/libmagic.patch | 181 +++++++++++++++++------------ ext/fileinfo/libmagic/apprentice.c | 38 +++--- 4 files changed, 142 insertions(+), 107 deletions(-) diff --git a/NEWS b/NEWS index 475eec61609..eca66987eb7 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,10 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? 2012, PHP 5.4.9 +- Fileinfo: + . Fixed bug #63248 (Load multiple magic files from a directory under Windows). + (Anatoliy) + ?? ??? 2012, PHP 5.4.8 - CLI server: diff --git a/ext/fileinfo/config.w32 b/ext/fileinfo/config.w32 index 46b87b56dcb..873a12c2f44 100644 --- a/ext/fileinfo/config.w32 +++ b/ext/fileinfo/config.w32 @@ -4,22 +4,16 @@ ARG_ENABLE("fileinfo", "fileinfo support", "no"); if (PHP_FILEINFO != 'no') { - if (CHECK_HEADER_ADD_INCLUDE("dirent.h", "CFLAGS_FILEINFO") && - CHECK_LIB("dirent_a.lib", "fileinfo", PHP_FILEINFO)) { - LIBMAGIC_SOURCES=" apprentice.c apptype.c ascmagic.c \ - cdf.c cdf_time.c compress.c \ - encoding.c fsmagic.c funcs.c \ - is_tar.c magic.c print.c \ - readcdf.c readelf.c softmagic.c"; + LIBMAGIC_SOURCES=" apprentice.c apptype.c ascmagic.c \ + cdf.c cdf_time.c compress.c \ + encoding.c fsmagic.c funcs.c \ + is_tar.c magic.c print.c \ + readcdf.c readelf.c softmagic.c"; - if (VCVERS < 1500) { - ADD_FLAG('CFLAGS', '/Zm1000'); - } + if (VCVERS < 1500) { + ADD_FLAG('CFLAGS', '/Zm1000'); + } - EXTENSION('fileinfo', 'fileinfo.c', true, "/I" + configure_module_dirname + "/libmagic /I" + configure_module_dirname); - ADD_SOURCES(configure_module_dirname + '\\libmagic', LIBMAGIC_SOURCES, "fileinfo"); - } else { - WARNING("fileinfo not enabled; libraries and headers not found"); - PHP_FILEINFO = "no"; - } + EXTENSION('fileinfo', 'fileinfo.c', true, "/I" + configure_module_dirname + "/libmagic /I" + configure_module_dirname); + ADD_SOURCES(configure_module_dirname + '\\libmagic', LIBMAGIC_SOURCES, "fileinfo"); } diff --git a/ext/fileinfo/libmagic.patch b/ext/fileinfo/libmagic.patch index 15f6a6dadd1..ecb178ffa96 100644 --- a/ext/fileinfo/libmagic.patch +++ b/ext/fileinfo/libmagic.patch @@ -1,6 +1,6 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c ---- libmagic.origin/apprentice.c 2012-09-11 11:09:26.000000000 +0800 -+++ libmagic/apprentice.c 2012-09-11 11:36:51.000000000 +0800 +--- libmagic.origin/apprentice.c Sat Dec 17 18:17:18 2011 ++++ libmagic/apprentice.c Tue Oct 16 10:21:49 2012 @@ -29,6 +29,8 @@ * apprentice - make one pass through /etc/magic, learning its secrets. */ @@ -10,7 +10,7 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c #include "file.h" #ifndef lint -@@ -36,18 +38,34 @@ +@@ -36,18 +38,31 @@ #endif /* lint */ #include "magic.h" @@ -43,13 +43,11 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c -#ifdef QUICK -#include -#endif -+#ifndef PHP_WIN32 - #include -+#endif +-#include #define EATAB {while (isascii((unsigned char) *l) && \ isspace((unsigned char) *l)) ++l;} -@@ -112,12 +130,10 @@ +@@ -112,12 +127,10 @@ private int parse_strength(struct magic_set *, struct magic_entry *, const char *); private int parse_apple(struct magic_set *, struct magic_entry *, const char *); @@ -62,7 +60,7 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c private struct { const char *name; size_t len; -@@ -131,38 +147,7 @@ +@@ -131,38 +144,7 @@ { NULL, 0, NULL } }; @@ -102,7 +100,7 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c static const struct type_tbl_s { const char name[16]; -@@ -218,6 +203,10 @@ +@@ -218,6 +200,10 @@ # undef XX_NULL }; @@ -113,7 +111,7 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c private int get_type(const char *l, const char **t) { -@@ -275,15 +264,17 @@ +@@ -275,15 +261,17 @@ if (rv != 0) return -1; rv = apprentice_compile(ms, &magic, &nmagic, fn); @@ -136,7 +134,7 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c if (rv != 0) return -1; } -@@ -295,11 +286,7 @@ +@@ -295,11 +283,7 @@ return -1; } @@ -149,7 +147,7 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c ml->magic = magic; ml->nmagic = nmagic; -@@ -318,7 +305,6 @@ +@@ -318,7 +302,6 @@ } return 0; @@ -157,7 +155,7 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c } protected void -@@ -327,22 +313,18 @@ +@@ -327,22 +310,18 @@ if (p == NULL) return; switch (type) { @@ -186,7 +184,7 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c default: abort(); } -@@ -355,23 +337,27 @@ +@@ -355,23 +334,27 @@ char *p, *mfn; int file_err, errs = -1; struct mlist *mlist; @@ -223,7 +221,7 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c mlist->next = mlist->prev = mlist; while (fn) { -@@ -385,13 +371,13 @@ +@@ -385,13 +368,13 @@ fn = p; } if (errs == -1) { @@ -240,7 +238,7 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c return mlist; } -@@ -524,6 +510,7 @@ +@@ -524,6 +507,7 @@ abort(); } @@ -248,7 +246,7 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c /* * Magic entries with no description get a bonus because they depend * on subsequent magic entries to print something. -@@ -539,8 +526,8 @@ +@@ -539,8 +523,8 @@ private int apprentice_sort(const void *a, const void *b) { @@ -259,7 +257,7 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c size_t sa = apprentice_magic_strength(ma->mp); size_t sb = apprentice_magic_strength(mb->mp); if (sa == sb) -@@ -671,12 +658,22 @@ +@@ -671,12 +655,22 @@ load_1(struct magic_set *ms, int action, const char *fn, int *errs, struct magic_entry **marray, uint32_t *marraycount) { @@ -286,7 +284,7 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c if (errno != ENOENT) file_error(ms, errno, "cannot read magic file `%s'", fn); -@@ -684,9 +681,12 @@ +@@ -684,9 +678,12 @@ return; } @@ -302,7 +300,7 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c if (len == 0) /* null line, garbage, etc */ continue; if (line[len - 1] == '\n') { -@@ -736,8 +736,7 @@ +@@ -736,8 +733,7 @@ break; } } @@ -312,7 +310,7 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c } /* -@@ -754,23 +753,19 @@ +@@ -754,23 +750,21 @@ apprentice_load(struct magic_set *ms, struct magic **magicp, uint32_t *nmagicp, const char *fn, int action) { @@ -325,8 +323,12 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c + size_t files = 0, maxfiles = 0; + char **filearr = NULL; struct stat st; - DIR *dir; - struct dirent *d; +- DIR *dir; +- struct dirent *d; ++ php_stream *dir; ++ php_stream_dirent d; ++ ++ TSRMLS_FETCH(); ms->flags |= MAGIC_CHECK; /* Enable checks for parsed files */ @@ -341,28 +343,33 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c marraycount = 0; /* print silly verbose header for USG compat. */ -@@ -778,14 +773,18 @@ +@@ -778,22 +772,26 @@ (void)fprintf(stderr, "%s\n", usg_hdr); /* load directory or file */ - if (stat(fn, &st) == 0 && S_ISDIR(st.st_mode)) { +- dir = opendir(fn); + /* FIXME: Read file names and sort them to prevent + non-determinism. See Debian bug #488562. */ + if (php_sys_stat(fn, &st) == 0 && S_ISDIR(st.st_mode)) { -+ int mflen; -+ char mfn[MAXPATHLEN]; - dir = opendir(fn); ++ int mflen; ++ char mfn[MAXPATHLEN]; ++ ++ dir = php_stream_opendir(fn, REPORT_ERRORS, NULL); if (!dir) { errs++; goto out; } - while ((d = readdir(dir)) != NULL) { +- while ((d = readdir(dir)) != NULL) { - if (asprintf(&mfn, "%s/%s", fn, d->d_name) < 0) { -+ if ((mflen = snprintf(mfn, sizeof(mfn), "%s/%s", fn, d->d_name)) < 0) { ++ while (php_stream_readdir(dir, &d)) { ++ if ((mflen = snprintf(mfn, sizeof(mfn), "%s/%s", fn, d.d_name)) < 0) { file_oomem(ms, - strlen(fn) + strlen(d->d_name) + 2); +- strlen(fn) + strlen(d->d_name) + 2); ++ strlen(fn) + strlen(d.d_name) + 2); errs++; -@@ -793,7 +792,6 @@ +- closedir(dir); ++ php_stream_closedir(dir); goto out; } if (stat(mfn, &st) == -1 || !S_ISREG(st.st_mode)) { @@ -375,7 +382,8 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c realloc(filearr, mlen))) == NULL) { file_oomem(ms, mlen); - free(mfn); - closedir(dir); +- closedir(dir); ++ php_stream_closedir(dir); errs++; goto out; } @@ -383,7 +391,8 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c - filearr[files++] = mfn; + filearr[files++] = estrndup(mfn, (mflen > sizeof(mfn) - 1)? sizeof(mfn) - 1: mflen); } - closedir(dir); +- closedir(dir); ++ php_stream_closedir(dir); qsort(filearr, files, sizeof(*filearr), cmpstrp); for (i = 0; i < files; i++) { load_1(ms, action, filearr[i], &errs, &marray, @@ -512,7 +521,7 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c m->mimetype[0] = '\0'; /* initialise MIME type to none */ if (m->cont_level == 0) ++(*nmentryp); /* make room for next */ -@@ -2195,56 +2180,69 @@ +@@ -2195,56 +2180,79 @@ /* * handle a compiled file. @@ -543,6 +552,16 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c + ret = 3; + goto internal_loaded; + } ++ ++#ifdef PHP_WIN32 ++ /* Don't bother on windows with php_stream_open_wrapper, ++ return to give apprentice_load() a chance. */ ++ if (php_stream_stat_path_ex(fn, 0, &st, NULL) == SUCCESS) { ++ if (st.sb.st_mode & S_IFDIR) { ++ goto error2; ++ } ++ } ++#endif dbname = mkdbname(ms, fn, 0); if (dbname == NULL) @@ -605,7 +624,7 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c ptr = (uint32_t *)(void *)*magicp; if (*ptr != MAGICNO) { if (swap4(*ptr) != MAGICNO) { -@@ -2259,35 +2257,55 @@ +@@ -2259,35 +2267,55 @@ else version = ptr[1]; if (version != VERSIONNO) { @@ -677,7 +696,7 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c return -1; } -@@ -2301,42 +2319,49 @@ +@@ -2301,42 +2329,49 @@ apprentice_compile(struct magic_set *ms, struct magic **magicp, uint32_t *nmagicp, const char *fn) { @@ -738,7 +757,7 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c return rv; } -@@ -2349,6 +2374,7 @@ +@@ -2349,6 +2384,7 @@ { const char *p, *q; char *buf; @@ -746,7 +765,7 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c if (strip) { if ((p = strrchr(fn, '/')) != NULL) -@@ -2370,14 +2396,14 @@ +@@ -2370,14 +2406,14 @@ q++; /* Compatibility with old code that looked in .mime */ if (ms->flags & MAGIC_MIME) { @@ -765,7 +784,7 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c /* Compatibility with old code that looked in .mime */ if (strstr(p, ".mime") != NULL) -@@ -2467,7 +2493,7 @@ +@@ -2467,7 +2503,7 @@ m->offset = swap4((uint32_t)m->offset); m->in_offset = swap4((uint32_t)m->in_offset); m->lineno = swap4((uint32_t)m->lineno); @@ -775,8 +794,8 @@ diff -u libmagic.origin/apprentice.c libmagic/apprentice.c m->str_flags = swap4(m->str_flags); } diff -u libmagic.origin/ascmagic.c libmagic/ascmagic.c ---- libmagic.origin/ascmagic.c 2012-09-11 11:09:26.000000000 +0800 -+++ libmagic/ascmagic.c 2012-09-11 11:33:55.000000000 +0800 +--- libmagic.origin/ascmagic.c Sat Dec 17 18:17:18 2011 ++++ libmagic/ascmagic.c Tue Apr 10 09:46:33 2012 @@ -139,10 +139,8 @@ /* malloc size is a conservative overestimate; could be improved, or at least realloced after conversion. */ @@ -801,8 +820,8 @@ diff -u libmagic.origin/ascmagic.c libmagic/ascmagic.c return rv; } diff -u libmagic.origin/cdf.c libmagic/cdf.c ---- libmagic.origin/cdf.c 2012-09-11 11:09:26.000000000 +0800 -+++ libmagic/cdf.c 2012-09-11 11:33:55.000000000 +0800 +--- libmagic.origin/cdf.c Mon Feb 20 23:35:29 2012 ++++ libmagic/cdf.c Tue Apr 10 09:46:33 2012 @@ -43,7 +43,17 @@ #include #endif @@ -865,8 +884,8 @@ diff -u libmagic.origin/cdf.c libmagic/cdf.c (void)fprintf(stderr, "timestamp %s\n", buf); } else { diff -u libmagic.origin/cdf.h libmagic/cdf.h ---- libmagic.origin/cdf.h 2012-09-11 11:09:26.000000000 +0800 -+++ libmagic/cdf.h 2012-09-11 11:33:55.000000000 +0800 +--- libmagic.origin/cdf.h Fri Feb 17 06:28:31 2012 ++++ libmagic/cdf.h Tue Apr 10 09:46:34 2012 @@ -35,7 +35,7 @@ #ifndef _H_CDF_ #define _H_CDF_ @@ -903,8 +922,8 @@ diff -u libmagic.origin/cdf.h libmagic/cdf.h void cdf_swap_header(cdf_header_t *); void cdf_unpack_header(cdf_header_t *, char *); diff -u libmagic.origin/cdf_time.c libmagic/cdf_time.c ---- libmagic.origin/cdf_time.c 2012-09-11 11:09:26.000000000 +0800 -+++ libmagic/cdf_time.c 2012-09-11 11:33:55.000000000 +0800 +--- libmagic.origin/cdf_time.c Tue Dec 13 14:48:41 2011 ++++ libmagic/cdf_time.c Tue Apr 10 09:46:34 2012 @@ -96,7 +96,7 @@ } @@ -962,8 +981,8 @@ diff -u libmagic.origin/cdf_time.c libmagic/cdf_time.c static const char *ref = "Sat Apr 23 01:30:00 1977"; char *p, *q; diff -u libmagic.origin/compress.c libmagic/compress.c ---- libmagic.origin/compress.c 2012-09-11 11:09:26.000000000 +0800 -+++ libmagic/compress.c 2012-09-11 11:33:55.000000000 +0800 +--- libmagic.origin/compress.c Sat Dec 17 18:17:18 2011 ++++ libmagic/compress.c Tue Apr 10 09:46:34 2012 @@ -32,6 +32,7 @@ * uncompress(method, old, n, newch) - uncompress old into new, * using method, return sizeof new @@ -1124,10 +1143,9 @@ diff -u libmagic.origin/compress.c libmagic/compress.c } -#endif +#endif /* if PHP_FILEINFO_UNCOMPRESS */ -Only in libmagic: diff diff -u libmagic.origin/file.h libmagic/file.h ---- libmagic.origin/file.h 2012-09-11 11:09:26.000000000 +0800 -+++ libmagic/file.h 2012-09-11 11:33:55.000000000 +0800 +--- libmagic.origin/file.h Tue Sep 20 17:30:14 2011 ++++ libmagic/file.h Mon Apr 23 17:58:54 2012 @@ -33,11 +33,9 @@ #ifndef __file_h__ #define __file_h__ @@ -1285,22 +1303,24 @@ diff -u libmagic.origin/file.h libmagic/file.h size_t strlcat(char *dst, const char *src, size_t siz); #endif #ifndef HAVE_GETLINE -@@ -500,4 +487,12 @@ - #define FILE_RCSID(id) +@@ -498,6 +485,14 @@ #endif - + #else + #define FILE_RCSID(id) ++#endif ++ +#ifdef PHP_WIN32 +#define FINFO_LSEEK_FUNC _lseek +#define FINFO_READ_FUNC _read +#else +#define FINFO_LSEEK_FUNC lseek +#define FINFO_READ_FUNC read -+#endif -+ + #endif + #endif /* __file_h__ */ diff -u libmagic.origin/fsmagic.c libmagic/fsmagic.c ---- libmagic.origin/fsmagic.c 2012-09-11 11:09:26.000000000 +0800 -+++ libmagic/fsmagic.c 2012-09-11 11:33:55.000000000 +0800 +--- libmagic.origin/fsmagic.c Tue Aug 23 10:57:10 2011 ++++ libmagic/fsmagic.c Tue Apr 10 09:46:34 2012 @@ -59,27 +59,21 @@ # define minor(dev) ((dev) & 0xff) #endif @@ -1511,10 +1531,10 @@ diff -u libmagic.origin/fsmagic.c libmagic/fsmagic.c -#else - if (file_printf(ms, "block special") == -1) - return -1; --#endif + #endif - } - return 1; - #endif +-#endif - /* TODO add code to handle V7 MUX and Blit MUX files */ + #ifdef S_IFIFO @@ -1624,8 +1644,8 @@ diff -u libmagic.origin/fsmagic.c libmagic/fsmagic.c /* diff -u libmagic.origin/funcs.c libmagic/funcs.c ---- libmagic.origin/funcs.c 2012-09-11 11:09:26.000000000 +0800 -+++ libmagic/funcs.c 2012-09-11 11:33:55.000000000 +0800 +--- libmagic.origin/funcs.c Sat Dec 17 18:17:18 2011 ++++ libmagic/funcs.c Mon Apr 23 17:58:54 2012 @@ -41,52 +41,42 @@ #if defined(HAVE_WCTYPE_H) #include @@ -1920,8 +1940,8 @@ diff -u libmagic.origin/funcs.c libmagic/funcs.c } + diff -u libmagic.origin/magic.c libmagic/magic.c ---- libmagic.origin/magic.c 2012-09-11 11:09:26.000000000 +0800 -+++ libmagic/magic.c 2012-09-11 11:33:55.000000000 +0800 +--- libmagic.origin/magic.c Thu May 26 03:27:59 2011 ++++ libmagic/magic.c Tue Apr 10 09:46:34 2012 @@ -25,11 +25,6 @@ * SUCH DAMAGE. */ @@ -2298,8 +2318,8 @@ diff -u libmagic.origin/magic.c libmagic/magic.c public const char * magic_error(struct magic_set *ms) diff -u libmagic.origin/magic.h libmagic/magic.h ---- libmagic.origin/magic.h 2012-09-11 11:09:26.000000000 +0800 -+++ libmagic/magic.h 2012-09-11 11:33:55.000000000 +0800 +--- libmagic.origin/magic.h Sun Dec 18 15:54:43 2011 ++++ libmagic/magic.h Tue Apr 10 09:46:34 2012 @@ -85,6 +85,7 @@ const char *magic_getpath(const char *, int); @@ -2317,9 +2337,9 @@ diff -u libmagic.origin/magic.h libmagic/magic.h int magic_errno(magic_t); diff -u libmagic.origin/print.c libmagic/print.c ---- libmagic.origin/print.c 2012-09-11 11:09:26.000000000 +0800 -+++ libmagic/print.c 2012-09-11 11:33:55.000000000 +0800 -@@ -29,6 +29,9 @@ +--- libmagic.origin/print.c Tue Sep 20 17:28:09 2011 ++++ libmagic/print.c Tue Oct 16 10:13:39 2012 +@@ -29,12 +29,16 @@ * print.c - debugging printout routines */ @@ -2329,7 +2349,14 @@ diff -u libmagic.origin/print.c libmagic/print.c #include "file.h" #ifndef lint -@@ -46,174 +49,21 @@ + FILE_RCSID("@(#)$File: print.c,v 1.71 2011/09/20 15:28:09 christos Exp $") + #endif /* lint */ + ++#include + #include + #include + #include +@@ -45,174 +49,21 @@ #define SZOF(a) (sizeof(a) / sizeof(a[0])) @@ -2512,8 +2539,8 @@ diff -u libmagic.origin/print.c libmagic/print.c protected const char * diff -u libmagic.origin/readcdf.c libmagic/readcdf.c ---- libmagic.origin/readcdf.c 2012-09-11 11:09:26.000000000 +0800 -+++ libmagic/readcdf.c 2012-09-11 11:33:55.000000000 +0800 +--- libmagic.origin/readcdf.c Mon Feb 20 21:04:58 2012 ++++ libmagic/readcdf.c Tue Apr 10 09:46:34 2012 @@ -30,7 +30,11 @@ #endif @@ -2560,8 +2587,8 @@ diff -u libmagic.origin/readcdf.c libmagic/readcdf.c if ((ec = strchr(c, '\n')) != NULL) *ec = '\0'; diff -u libmagic.origin/readelf.c libmagic/readelf.c ---- libmagic.origin/readelf.c 2012-09-11 11:09:26.000000000 +0800 -+++ libmagic/readelf.c 2012-09-11 11:33:55.000000000 +0800 +--- libmagic.origin/readelf.c Tue Aug 23 10:57:10 2011 ++++ libmagic/readelf.c Tue Apr 10 09:46:34 2012 @@ -49,7 +49,7 @@ off_t, int *, int); private int doshn(struct magic_set *, int, int, int, off_t, int, size_t, @@ -2717,8 +2744,8 @@ diff -u libmagic.origin/readelf.c libmagic/readelf.c if (fstat(fd, &st) == -1) { diff -u libmagic.origin/softmagic.c libmagic/softmagic.c ---- libmagic.origin/softmagic.c 2012-09-11 11:09:26.000000000 +0800 -+++ libmagic/softmagic.c 2012-09-11 11:33:55.000000000 +0800 +--- libmagic.origin/softmagic.c Sat Dec 17 18:17:18 2011 ++++ libmagic/softmagic.c Fri May 25 09:59:25 2012 @@ -41,6 +41,11 @@ #include #include diff --git a/ext/fileinfo/libmagic/apprentice.c b/ext/fileinfo/libmagic/apprentice.c index d11bd159a88..787eb793678 100644 --- a/ext/fileinfo/libmagic/apprentice.c +++ b/ext/fileinfo/libmagic/apprentice.c @@ -63,9 +63,6 @@ FILE_RCSID("@(#)$File: apprentice.c,v 1.173 2011/12/08 12:38:24 rrt Exp $") #include #include #include -#ifndef PHP_WIN32 -#include -#endif #define EATAB {while (isascii((unsigned char) *l) && \ isspace((unsigned char) *l)) ++l;} @@ -759,8 +756,10 @@ apprentice_load(struct magic_set *ms, struct magic **magicp, uint32_t *nmagicp, size_t files = 0, maxfiles = 0; char **filearr = NULL; struct stat st; - DIR *dir; - struct dirent *d; + php_stream *dir; + php_stream_dirent d; + + TSRMLS_FETCH(); ms->flags |= MAGIC_CHECK; /* Enable checks for parsed files */ @@ -776,19 +775,20 @@ apprentice_load(struct magic_set *ms, struct magic **magicp, uint32_t *nmagicp, /* FIXME: Read file names and sort them to prevent non-determinism. See Debian bug #488562. */ if (php_sys_stat(fn, &st) == 0 && S_ISDIR(st.st_mode)) { - int mflen; - char mfn[MAXPATHLEN]; - dir = opendir(fn); + int mflen; + char mfn[MAXPATHLEN]; + + dir = php_stream_opendir(fn, REPORT_ERRORS, NULL); if (!dir) { errs++; goto out; } - while ((d = readdir(dir)) != NULL) { - if ((mflen = snprintf(mfn, sizeof(mfn), "%s/%s", fn, d->d_name)) < 0) { + while (php_stream_readdir(dir, &d)) { + if ((mflen = snprintf(mfn, sizeof(mfn), "%s/%s", fn, d.d_name)) < 0) { file_oomem(ms, - strlen(fn) + strlen(d->d_name) + 2); + strlen(fn) + strlen(d.d_name) + 2); errs++; - closedir(dir); + php_stream_closedir(dir); goto out; } if (stat(mfn, &st) == -1 || !S_ISREG(st.st_mode)) { @@ -801,14 +801,14 @@ apprentice_load(struct magic_set *ms, struct magic **magicp, uint32_t *nmagicp, if ((filearr = CAST(char **, realloc(filearr, mlen))) == NULL) { file_oomem(ms, mlen); - closedir(dir); + php_stream_closedir(dir); errs++; goto out; } } filearr[files++] = estrndup(mfn, (mflen > sizeof(mfn) - 1)? sizeof(mfn) - 1: mflen); } - closedir(dir); + php_stream_closedir(dir); qsort(filearr, files, sizeof(*filearr), cmpstrp); for (i = 0; i < files; i++) { load_1(ms, action, filearr[i], &errs, &marray, @@ -2206,6 +2206,16 @@ apprentice_map(struct magic_set *ms, struct magic **magicp, uint32_t *nmagicp, goto internal_loaded; } +#ifdef PHP_WIN32 + /* Don't bother on windows with php_stream_open_wrapper, + return to give apprentice_load() a chance. */ + if (php_stream_stat_path_ex(fn, 0, &st, NULL) == SUCCESS) { + if (st.sb.st_mode & S_IFDIR) { + goto error2; + } + } +#endif + dbname = mkdbname(ms, fn, 0); if (dbname == NULL) goto error2;