Skip to content

Commit ec4e9d2

Browse files
committed
minor symfony#14028 [Security] [Core] String utils refactor (sarciszewski, ircmaxell)
This PR was submitted for the 2.7 branch but it was merged into the 2.3 branch instead (closes symfony#14028). Discussion ---------- [Security] [Core] String utils refactor | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | This supersedes symfony#13984 (it includes it, but also includes additional refactoring). Since length information is leaked in any case, preventing unnecessary duplication of secrets is important. Since casting will *always* make a copy, we only cast if absolutely necessary. Additionally, appending will create a new copy of the secret, so we avoid doing that, but instead only iterate over the minimum of the two strings. Commits ------- 45cfb44 Change behavior to mirror hash_equals() returning early if there is a length mismatch 8269589 CS fixing bdea4ba Prevent modifying secrets as much as possible 76b36d3 Update StringUtils.php 7221efc Whitespace 56ed71c Update StringUtils.php
2 parents 36948bb + 45cfb44 commit ec4e9d2

File tree

1 file changed

+37
-11
lines changed

1 file changed

+37
-11
lines changed

src/Symfony/Component/Security/Core/Util/StringUtils.php

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,29 +38,55 @@ private function __construct()
3838
*/
3939
public static function equals($knownString, $userInput)
4040
{
41-
$knownString = (string) $knownString;
42-
$userInput = (string) $userInput;
41+
// Avoid making unnecessary duplications of secret data
42+
if (!is_string($knownString)) {
43+
$knownString = (string) $knownString;
44+
}
45+
46+
if (!is_string($userInput)) {
47+
$userInput = (string) $userInput;
48+
}
4349

4450
if (function_exists('hash_equals')) {
4551
return hash_equals($knownString, $userInput);
4652
}
4753

48-
$knownLen = strlen($knownString);
49-
$userLen = strlen($userInput);
54+
$knownLen = self::safeStrlen($knownString);
55+
$userLen = self::safeStrlen($userInput);
5056

51-
// Extend the known string to avoid uninitialized string offsets
52-
$knownString .= $userInput;
57+
if ($userLen != $knownLen) {
58+
return false;
59+
}
5360

54-
// Set the result to the difference between the lengths
55-
$result = $knownLen - $userLen;
61+
$result = 0;
5662

57-
// Note that we ALWAYS iterate over the user-supplied length
58-
// This is to mitigate leaking length information
59-
for ($i = 0; $i < $userLen; $i++) {
63+
for ($i = 0; $i < $knownLen; $i++) {
6064
$result |= (ord($knownString[$i]) ^ ord($userInput[$i]));
6165
}
6266

6367
// They are only identical strings if $result is exactly 0...
6468
return 0 === $result;
6569
}
70+
71+
/**
72+
* Return the number of bytes in a string
73+
*
74+
* @param string $string The string whose length we wish to obtain
75+
* @return int
76+
*/
77+
public static function safeStrlen($string)
78+
{
79+
// Premature optimization
80+
// Since this cannot be changed at runtime, we can cache it
81+
static $func_exists = null;
82+
if ($func_exists === null) {
83+
$func_exists = function_exists('mb_strlen');
84+
}
85+
86+
if ($func_exists) {
87+
return mb_strlen($string, '8bit');
88+
}
89+
90+
return strlen($string);
91+
}
6692
}

0 commit comments

Comments
 (0)