7 ms·

> NOTE In general, it's not possible to prevent length leaks. So it's OK to leak the length. I would imagine that you can prevent length leaks by looping throu

by umsm 12y ago

> NOTE In general, it's not possible to prevent length leaks. So it's OK to leak the length. I would imagine that you can prevent length leaks by looping through the characters of the known value and then returning that comparison with an additional check of length. function timingSafeEquals($safe, $user) { $safeLen = strlen($safe); $userLen = strlen($user); $result = 0; for ($i = 0; $i < $userLen; $i++) { $result |= (ord($safe[$i]) ^ ord($user[$i])); } return $result === 0 && $userLen === $safeLen; }

totony 12y ago
The for loop duration here will vary depending on the length of the string
- umsm 12y ago
  That was a quick copy/paste. But imagine the loop would be the string we know (i.e. the password). Looping through the 10-character pass should be identical every time, regardless of what the user entered.
  - totony 12y ago
    I imagine the strlen() function is not constant-timed, so I'd remove it, but it might work yes (but you'd have to check PHP's source code and have a deep understanding of it to really be sure).
    - mappu 12y ago
      libc `strlen()` is O(n), but PHP strings are binary safe (i.e. http://3v4l.org/47iaP http://3v4l.org/47iaP ) so it must be storing a length as part of the string zval. So PHP strlen should be constant time. +1 for recommending checking the PHP source code to be sure, though
    - TazeTSchnitzel 12y ago
      strlen() in PHP is constant-time O(1), we don't use C strings. (Well, we do, but we store length information and reference count them) See: http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_builtin_functions.c#524 http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_builtin_function...
Xylakant 12y ago
If you follow the link in the post you'll find a pretty convincing explanation why it's not possible to prevent leaking the length in the general case. I'm not certain that this applies to PHP code as well, since the problem is pretty low level. However, leaking the length is much less of a problem than allowing the attacker to guess characters one by one.
ircmaxell 12y ago
So, I tried this in the past. The problem you'll run into with PHP specifically is that reading an undefined string offset (past the end) will result in a notice: http://3v4l.org/nIkf5 http://3v4l.org/nIkf5 Which means that errors are triggered. So you can increase the length of the user string and note a linear increase in runtime until you increase it past the length of the string, at which point it becomes MUCH slower on a per-character basis (even if you don't do anything with the notice, the error mechanism is still triggered internally, which isn't cheap). Actually, my original code was more robust as it never read past the end of the string, preventing the notice: /** * A timing safe equals comparison * * To prevent leaking length information, it is important * that user input is always used as the second parameter. * * @param string $safe The internal (safe) value to be checked * @param string $user The user submitted (unsafe) value * * @return boolean True if the two strings are identical. */ function timingSafeEquals($safe, $user) { // Prevent issues if string length is 0 $safe .= chr(0); $user .= chr(0); $safeLen = strlen($safe); $userLen = strlen($user); // Set the result to the difference between the lengths $result = $safeLen - $userLen; // Note that we ALWAYS iterate over the user-supplied length // This is to prevent leaking length information for ($i = 0; $i < $userLen; $i++) { // Using % here is a trick to prevent notices // It's safe, since if the lengths are different // $result is already non-0 $result |= (ord($safe[$i % $safeLen]) ^ ord($user[$i])); } // They are only identical strings if $result is exactly 0... return $result === 0; } There are a few problems here though that are non-trivial as are explained in the post: http://security.stackexchange.com/questions/49849/timing-safe-string-comparison-avoiding-length-leak http://security.stackexchange.com/questions/49849/timing-saf... Basically, while it may keep the length %64 safe (since cache lines are 64 bites wide), it doesn't keep the length safe in general. Some length information will be leaked on larger strings. And considering it's impossible to protect the length in the general case, making a function which says it protects length is a lie. Therefore I don't even try and hence save the complexity. But let me ask this: what cases would you have where are you trying to protect the length? Anything with variable length input (like a password) should likely be one-way hashed anyway. So you'd be comparing fixed-length hashes. So where's the possible leak?

by umsm 12y ago

totony 12y ago

The for loop duration here will vary depending on the length of the string

umsm 12y ago
That was a quick copy/paste. But imagine the loop would be the string we know (i.e. the password). Looping through the 10-character pass should be identical every time, regardless of what the user entered.
- totony 12y ago
  I imagine the strlen() function is not constant-timed, so I'd remove it, but it might work yes (but you'd have to check PHP's source code and have a deep understanding of it to really be sure).
  - mappu 12y ago
    libc `strlen()` is O(n), but PHP strings are binary safe (i.e. http://3v4l.org/47iaP http://3v4l.org/47iaP ) so it must be storing a length as part of the string zval. So PHP strlen should be constant time. +1 for recommending checking the PHP source code to be sure, though
  - TazeTSchnitzel 12y ago
    strlen() in PHP is constant-time O(1), we don't use C strings. (Well, we do, but we store length information and reference count them) See: http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_builtin_functions.c#524 http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_builtin_function...

Xylakant 12y ago

If you follow the link in the post you'll find a pretty convincing explanation why it's not possible to prevent leaking the length in the general case. I'm not certain that this applies to PHP code as well, since the problem is pretty low level. However, leaking the length is much less of a problem than allowing the attacker to guess characters one by one.

ircmaxell 12y ago

So, I tried this in the past. The problem you'll run into with PHP specifically is that reading an undefined string offset (past the end) will result in a notice: http://3v4l.org/nIkf5 http://3v4l.org/nIkf5 Which means that errors are triggered. So you can increase the length of the user string and note a linear increase in runtime until you increase it past the length of the string, at which point it becomes MUCH slower on a per-character basis (even if you don't do anything with the notice, the error mechanism is still triggered internally, which isn't cheap). Actually, my original code was more robust as it never read past the end of the string, preventing the notice: /** * A timing safe equals comparison * * To prevent leaking length information, it is important * that user input is always used as the second parameter. * * @param string $safe The internal (safe) value to be checked * @param string $user The user submitted (unsafe) value * * @return boolean True if the two strings are identical. */ function timingSafeEquals($safe, $user) { // Prevent issues if string length is 0 $safe .= chr(0); $user .= chr(0); $safeLen = strlen($safe); $userLen = strlen($user); // Set the result to the difference between the lengths $result = $safeLen - $userLen; // Note that we ALWAYS iterate over the user-supplied length // This is to prevent leaking length information for ($i = 0; $i < $userLen; $i++) { // Using % here is a trick to prevent notices // It's safe, since if the lengths are different // $result is already non-0 $result |= (ord($safe[$i % $safeLen]) ^ ord($user[$i])); } // They are only identical strings if $result is exactly 0... return $result === 0; } There are a few problems here though that are non-trivial as are explained in the post: http://security.stackexchange.com/questions/49849/timing-safe-string-comparison-avoiding-length-leak http://security.stackexchange.com/questions/49849/timing-saf... Basically, while it may keep the length %64 safe (since cache lines are 64 bites wide), it doesn't keep the length safe in general. Some length information will be leaked on larger strings. And considering it's impossible to protect the length in the general case, making a function which says it protects length is a lie. Therefore I don't even try and hence save the complexity. But let me ask this: what cases would you have where are you trying to protect the length? Anything with variable length input (like a password) should likely be one-way hashed anyway. So you'd be comparing fixed-length hashes. So where's the possible leak?