Commit: 1751d5fabeff466f08da560caa6f92222ade5a82 Author: Anthony Ferrara <ircmax...@gmail.com> Sat, 6 Oct 2012 10:38:41 -0400 Parents: 25b2d364e995fc070ae16ee34f60d25148413769 Branches: master
Link: http://git.php.net/?p=php-src.git;a=commitdiff;h=1751d5fabeff466f08da560caa6f92222ade5a82 Log: Really fix leaks, add test cases to prove it... Changed paths: M ext/standard/password.c M ext/standard/tests/password/password_bcrypt_errors.phpt M ext/standard/tests/password/password_hash_error.phpt M ext/standard/tests/password/password_needs_rehash.phpt Diff: diff --git a/ext/standard/password.c b/ext/standard/password.c index af42a6f..9667fdc 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 f36d11f..2548c9a 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 695a6c4..952250c 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 2fc3983..734729e 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! -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
