diff --git a/ext/standard/password.c b/ext/standard/password.c index 9d6bd6f2003d..a6fe1126d4e6 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -113,30 +113,54 @@ static zend_string* php_password_get_salt(zval *unused_, size_t required_salt_le /* bcrypt implementation */ -static bool php_password_bcrypt_get_cost(const zend_string *hash, zend_long *cost) { +static bool php_password_bcrypt_get_cost(zend_array *options, zend_long *cost) { + zval *zcost; + if (options && (zcost = zend_hash_str_find(options, "cost", sizeof("cost")-1)) != NULL) { + *cost = zval_get_long(zcost); + return *cost >= 4 && *cost <= 31; + } + + *cost = PHP_PASSWORD_BCRYPT_COST; + return true; +} + +static bool php_password_bcrypt_parse(const zend_string *hash, zend_long *cost) { const char *h = ZSTR_VAL(hash); - if ((ZSTR_LEN(hash) != 60) || - (h[0] != '$') || (h[1] != '2') || (h[2] != 'y') || (h[3] != '$') || - !ZEND_IS_DIGIT(h[4]) || !ZEND_IS_DIGIT(h[5]) || (h[6] != '$')) { + if (ZSTR_LEN(hash) != 60 || + h[0] != '$' || h[1] != '2' || h[2] != 'y' || h[3] != '$' || + !ZEND_IS_DIGIT(h[4]) || !ZEND_IS_DIGIT(h[5]) || h[6] != '$' || + (h[28] != '.' && h[28] != 'O' && h[28] != 'e' && h[28] != 'u') + ) { return false; } - *cost = (h[4] - '0') * 10 + (h[5] - '0'); + unsigned char c = (h[4] - '0') * 10 + (h[5] - '0'); + if (c < 4 || c > 31) { + return false; + } + + for (size_t i = 7; i < 60; i++) { + if (h[i] < '.' || h[i] > 'z' || (h[i] > '9' && h[i] < 'A') || (h[i] > 'Z' && h[i] < 'a')) { + return false; + } + } + + if (cost != NULL) { + *cost = (zend_long) c; + } - return *cost >= 4 && *cost <= 31; + return true; } static bool php_password_bcrypt_valid(const zend_string *hash) { - zend_long cost; - - return php_password_bcrypt_get_cost(hash, &cost); + return php_password_bcrypt_parse(hash, NULL); } static int php_password_bcrypt_get_info(zval *return_value, const zend_string *hash) { zend_long cost; - if (!php_password_bcrypt_get_cost(hash, &cost)) { + if (!php_password_bcrypt_parse(hash, &cost)) { /* Should never get called this way. */ return FAILURE; } @@ -146,17 +170,14 @@ static int php_password_bcrypt_get_info(zval *return_value, const zend_string *h } static bool php_password_bcrypt_needs_rehash(const zend_string *hash, zend_array *options) { - zval *znew_cost; - zend_long old_cost; - zend_long new_cost = PHP_PASSWORD_BCRYPT_COST; + zend_long old_cost, new_cost; - if (!php_password_bcrypt_get_cost(hash, &old_cost)) { + if (!php_password_bcrypt_parse(hash, &old_cost)) { /* Should never get called this way. */ return 1; } - if (options && (znew_cost = zend_hash_str_find(options, "cost", sizeof("cost")-1)) != NULL) { - new_cost = zval_get_long(znew_cost); - } + + php_password_bcrypt_get_cost(options, &new_cost); return old_cost != new_cost; } @@ -188,19 +209,14 @@ static zend_string* php_password_bcrypt_hash(const zend_string *password, zend_a char hash_format[10]; size_t hash_format_len; zend_string *result, *hash, *salt; - zval *zcost; - zend_long cost = PHP_PASSWORD_BCRYPT_COST; + zend_long cost; if (memchr(ZSTR_VAL(password), '\0', ZSTR_LEN(password))) { zend_value_error("Bcrypt password must not contain null character"); return NULL; } - if (options && (zcost = zend_hash_str_find(options, "cost", sizeof("cost")-1)) != NULL) { - cost = zval_get_long(zcost); - } - - if (cost < 4 || cost > 31) { + if (!php_password_bcrypt_get_cost(options, &cost)) { zend_value_error("Invalid bcrypt cost parameter specified: " ZEND_LONG_FMT, cost); return NULL; } diff --git a/ext/standard/tests/password/password_get_info.phpt b/ext/standard/tests/password/password_get_info.phpt index 29dd78fbc1f0..e8b153db352b 100644 --- a/ext/standard/tests/password/password_get_info.phpt +++ b/ext/standard/tests/password/password_get_info.phpt @@ -9,6 +9,7 @@ var_dump(password_get_info('$2y$10$MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR.oP7XOR9HD/vj var_dump(password_get_info('$2y$11$MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR.oP7XOR9HD/vjUuOj100y')); // Test Bcrypt Invalid Length var_dump(password_get_info('$2y$11$MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR.oP7XOR9HD/vjUuOj100')); +var_dump(password_get_info('$2y$11$MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR.oP7XOR9HD/vjUuOj1000y')); // Test Non-Bcrypt var_dump(password_get_info('$1$rasmusle$rISCgZzpwk3UhDidwXvin0')); @@ -26,7 +27,15 @@ $invalidHashes = [ 'below range' => '$2y$03$' . $suffix, 'above range' => '$2y$32$' . $suffix, 'missing separator' => '$2y$' . str_repeat('9', 56), + 'malformed salt segment' => '$2y$04$MTIzNDU2Nzg5MDEyMzQ1NEj0NmcAWSLR.oP7XOR9HD/vjUuOj100y', + 'malformed hash segment' => '$2y$04$MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR+oP7XOR9HD/vjUuOj100y', + 'malicious misalignment' => '$2y$4$MTIzNDU2Nzg5MDEyMzQ1Neej0NmcAWSLR+oP7XOR9HD/vjUuOj100y', + 'minus sign' => '$2y$-4$' . $suffix, + 'plus sign' => '$2y$+4$' . $suffix, + 'newline' => '$2y$'."\n".'4$' . $suffix, + 'space' => '$2y$ 4$' . $suffix, ]; + foreach ($invalidHashes as $description => $hash) { $info = password_get_info($hash); printf("%s: %s, options %d\n", $description, $info['algoName'], count($info['options'])); @@ -75,6 +84,15 @@ array(3) { array(0) { } } +array(3) { + ["algo"]=> + NULL + ["algoName"]=> + string(7) "unknown" + ["options"]=> + array(0) { + } +} valid 4: bcrypt, cost 4 valid 31: bcrypt, cost 31 non-digit: unknown, options 0 @@ -82,4 +100,11 @@ malformed separator: unknown, options 0 below range: unknown, options 0 above range: unknown, options 0 missing separator: unknown, options 0 +malformed salt segment: unknown, options 0 +malformed hash segment: unknown, options 0 +malicious misalignment: unknown, options 0 +minus sign: unknown, options 0 +plus sign: unknown, options 0 +newline: unknown, options 0 +space: unknown, options 0 OK! diff --git a/ext/standard/tests/password/password_needs_rehash.phpt b/ext/standard/tests/password/password_needs_rehash.phpt index f8390996f234..111822bca041 100644 --- a/ext/standard/tests/password/password_needs_rehash.phpt +++ b/ext/standard/tests/password/password_needs_rehash.phpt @@ -9,6 +9,10 @@ var_dump(password_needs_rehash('', PASSWORD_BCRYPT)); var_dump(password_needs_rehash('', 1)); var_dump(password_needs_rehash('', '2y')); +// Plausible-looking, but malformed hashes +var_dump(password_needs_rehash('$2y$04$MTIzNDU2Nzg5MDEyMzQ1NEj0NmcAWSLR.oP7XOR9HD/vjUuOj100y', PASSWORD_BCRYPT)); +var_dump(password_needs_rehash('$2y$04$MTIzNDU2Nzg5MDEyMzQ1Nej0NmcAWSLR+oP7XOR9HD/vjUuOj100y', PASSWORD_BCRYPT)); + // Valid, as it's an unknown algorithm var_dump(password_needs_rehash('', PASSWORD_DEFAULT)); var_dump(password_needs_rehash('', 0)); @@ -42,6 +46,10 @@ $costCases = [ 'malformed separator' => ['$2y$10x' . $suffix, 10], 'below range' => ['$2y$03$' . $suffix, 3], 'above range' => ['$2y$32$' . $suffix, 32], + 'minus sign' => ['$2y$-4$' . $suffix, 4], + 'plus sign' => ['$2y$+4$' . $suffix, 4], + 'newline' => ['$2y$'."\n".'4$' . $suffix, 4], + 'space' => ['$2y$ 4$' . $suffix, 4], ]; foreach ($costCases as $description => $case) { echo $description, ': '; @@ -63,6 +71,8 @@ bool(true) bool(true) bool(true) bool(true) +bool(true) +bool(true) bool(false) bool(false) bool(true) @@ -75,6 +85,10 @@ non-digit: bool(true) malformed separator: bool(true) below range: bool(true) above range: bool(true) +minus sign: bool(true) +plus sign: bool(true) +newline: bool(true) +space: bool(true) bool(true) bool(true) OK!