Skip to content

Commit 94ae35c

Browse files
nikicnikic
committed
Remove ability to specify 'salt' in password_hash()
1 parent 1870283 commit 94ae35c

File tree

6 files changed

+26
-131
lines changed

6 files changed

+26
-131
lines changed

‎UPGRADING‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,9 @@ PHP 8.0 UPGRADE NOTES
9797
string. Previously non-string needles were interpreted as an ASCII code
9898
point. An explicit call to chr() can be used to restore the previous
9999
behavior.
100+
. The 'salt' option of password_hash() is no longer supported. If the 'salt'
101+
option is used a warning is generated, the provided salt is ignored, and a
102+
generated salt is used instead.
100103

101104
- Zlib:
102105
. gzgetss() has been removed.

‎ext/standard/password.c‎

Lines changed: 3 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -52,19 +52,6 @@ void php_password_algo_unregister(const char *ident) {
5252
zend_hash_str_del(&php_password_algos, ident, strlen(ident));
5353
}
5454

55-
static int php_password_salt_is_alphabet(const char *str, const size_t len) /* {{{ */
56-
{
57-
size_t i = 0;
58-
59-
for (i = 0; i < len; i++) {
60-
if (!((str[i] >= 'A' && str[i] <= 'Z') || (str[i] >= 'a' && str[i] <= 'z') || (str[i] >= '0' && str[i] <= '9') || str[i] == '.' || str[i] == '/')) {
61-
return FAILURE;
62-
}
63-
}
64-
return SUCCESS;
65-
}
66-
/* }}} */
67-
6855
static int php_password_salt_to64(const char *str, const size_t str_len, const size_t out_len, char *ret) /* {{{ */
6956
{
7057
size_t pos = 0;
@@ -123,65 +110,11 @@ static zend_string* php_password_make_salt(size_t length) /* {{{ */
123110
/* }}} */
124111

125112
static zend_string* php_password_get_salt(zval *unused_, size_t required_salt_len, HashTable *options) {
126-
zend_string *buffer;
127-
zval *option_buffer;
128-
129-
if (!options || !(option_buffer = zend_hash_str_find(options, "salt", sizeof("salt") - 1))) {
130-
return php_password_make_salt(required_salt_len);
131-
}
132-
133-
php_error_docref(NULL, E_DEPRECATED, "Use of the 'salt' option to password_hash is deprecated");
134-
135-
switch (Z_TYPE_P(option_buffer)) {
136-
case IS_STRING:
137-
buffer = zend_string_copy(Z_STR_P(option_buffer));
138-
break;
139-
case IS_LONG:
140-
case IS_DOUBLE:
141-
case IS_OBJECT:
142-
buffer = zval_get_string(option_buffer);
143-
break;
144-
case IS_FALSE:
145-
case IS_TRUE:
146-
case IS_NULL:
147-
case IS_RESOURCE:
148-
case IS_ARRAY:
149-
default:
150-
php_error_docref(NULL, E_WARNING, "Non-string salt parameter supplied");
151-
return NULL;
152-
}
153-
154-
/* XXX all the crypt related APIs work with int for string length.
155-
That should be revised for size_t and then we maybe don't require
156-
the > INT_MAX check. */
157-
if (ZEND_SIZE_T_INT_OVFL(ZSTR_LEN(buffer))) {
158-
php_error_docref(NULL, E_WARNING, "Supplied salt is too long");
159-
zend_string_release_ex(buffer, 0);
160-
return NULL;
161-
}
162-
163-
if (ZSTR_LEN(buffer) < required_salt_len) {
164-
php_error_docref(NULL, E_WARNING, "Provided salt is too short: %zd expecting %zd", ZSTR_LEN(buffer), required_salt_len);
165-
zend_string_release_ex(buffer, 0);
166-
return NULL;
113+
if (options && zend_hash_str_exists(options, "salt", sizeof("salt") - 1)) {
114+
php_error_docref(NULL, E_WARNING, "The 'salt' option is no longer supported. The provided salt has been been ignored");
167115
}
168116

169-
if (php_password_salt_is_alphabet(ZSTR_VAL(buffer), ZSTR_LEN(buffer)) == FAILURE) {
170-
zend_string *salt = zend_string_alloc(required_salt_len, 0);
171-
if (php_password_salt_to64(ZSTR_VAL(buffer), ZSTR_LEN(buffer), required_salt_len, ZSTR_VAL(salt)) == FAILURE) {
172-
php_error_docref(NULL, E_WARNING, "Provided salt is too short: %zd", ZSTR_LEN(buffer));
173-
zend_string_release_ex(salt, 0);
174-
zend_string_release_ex(buffer, 0);
175-
return NULL;
176-
}
177-
zend_string_release_ex(buffer, 0);
178-
return salt;
179-
} else {
180-
zend_string *salt = zend_string_alloc(required_salt_len, 0);
181-
memcpy(ZSTR_VAL(salt), ZSTR_VAL(buffer), required_salt_len);
182-
zend_string_release_ex(buffer, 0);
183-
return salt;
184-
}
117+
return php_password_make_salt(required_salt_len);
185118
}
186119

187120
/* bcrypt implementation */

‎ext/standard/tests/password/password_bcrypt_errors.phpt‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -8,36 +8,10 @@ var_dump(password_hash("foo", PASSWORD_BCRYPT, array("cost" => 3)));
88

99
var_dump(password_hash("foo", PASSWORD_BCRYPT, array("cost" => 32)));
1010

11-
var_dump(password_hash("foo", PASSWORD_BCRYPT, array("salt" => "foo")));
12-
13-
var_dump(password_hash("foo", PASSWORD_BCRYPT, array("salt" => "123456789012345678901")));
14-
15-
var_dump(password_hash("foo", PASSWORD_BCRYPT, array("salt" => 123)));
16-
17-
var_dump(password_hash("foo", PASSWORD_BCRYPT, array("cost" => "foo")));
18-
1911
?>
2012
--EXPECTF--
2113
Warning: password_hash(): Invalid bcrypt cost parameter specified: 3 in %s on line %d
2214
NULL
2315

2416
Warning: password_hash(): Invalid bcrypt cost parameter specified: 32 in %s on line %d
2517
NULL
26-
27-
Deprecated: password_hash(): Use of the 'salt' option to password_hash is deprecated in %s on line %d
28-
29-
Warning: password_hash(): Provided salt is too short: 3 expecting 22 in %s on line %d
30-
NULL
31-
32-
Deprecated: password_hash(): Use of the 'salt' option to password_hash is deprecated in %s on line %d
33-
34-
Warning: password_hash(): Provided salt is too short: 21 expecting 22 in %s on line %d
35-
NULL
36-
37-
Deprecated: password_hash(): Use of the 'salt' option to password_hash is deprecated in %s on line %d
38-
39-
Warning: password_hash(): Provided salt is too short: 3 expecting 22 in %s on line %d
40-
NULL
41-
42-
Warning: password_hash(): Invalid bcrypt cost parameter specified: 0 in %s on line %d
43-
NULL

‎ext/standard/tests/password/password_deprecated_salts.phpt‎

Lines changed: 0 additions & 20 deletions
This file was deleted.

‎ext/standard/tests/password/password_hash_error.phpt‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,6 @@ var_dump(password_hash("foo", PASSWORD_BCRYPT, "baz"));
1616

1717
var_dump(password_hash(array(), PASSWORD_BCRYPT));
1818

19-
var_dump(password_hash("123", PASSWORD_BCRYPT, array("salt" => array())));
20-
21-
/* Non-string salt, checking for memory leaks */
22-
var_dump(password_hash('123', PASSWORD_BCRYPT, array('salt' => 1234)));
23-
2419
?>
2520
--EXPECTF--
2621
Warning: password_hash() expects at least 2 parameters, 0 given in %s on line %d
@@ -42,13 +37,3 @@ NULL
4237

4338
Warning: password_hash() expects parameter 1 to be string, array given in %s on line %d
4439
NULL
45-
46-
Deprecated: password_hash(): Use of the 'salt' option to password_hash is deprecated in %s on line %d
47-
48-
Warning: password_hash(): Non-string salt parameter supplied in %s on line %d
49-
NULL
50-
51-
Deprecated: password_hash(): Use of the 'salt' option to password_hash is deprecated in %s on line %d
52-
53-
Warning: password_hash(): Provided salt is too short: 4 expecting 22 in %s on line %d
54-
NULL

‎ext/standard/tests/password/password_removed_salt_option.phpt‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
--TEST--
2+
Test removed support for explicit salt option
3+
--FILE--
4+
<?php
5+
//-=-=-=-
6+
7+
8+
var_dump(strlen(password_hash("rasmuslerdorf", PASSWORD_BCRYPT, array("cost" => 7, "salt" => "usesomesillystringforsalt"))));
9+
10+
var_dump(strlen(password_hash("test", PASSWORD_BCRYPT, array("salt" => "123456789012345678901" . chr(0)))));
11+
12+
echo "OK!";
13+
?>
14+
--EXPECTF--
15+
Warning: password_hash(): The 'salt' option is no longer supported. The provided salt has been been ignored in %s on line %d
16+
int(60)
17+
18+
Warning: password_hash(): The 'salt' option is no longer supported. The provided salt has been been ignored in %s on line %d
19+
int(60)
20+
OK!

0 commit comments

Comments
 (0)