Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Improve timing safe comparison function

Improve the timing safe comparison function to better handle cases where input is of different length.

Note that it is now important to always pass any string that the user can directly control to the second parameter of the function. Otherwise, length information may be leaked.
  • Loading branch information...
commit 6ef999b2d566c59819f99e0eeab77940fbd0d77c 1 parent 1849d1e
Anthony Ferrara authored

Showing 1 changed file with 21 additions and 10 deletions. Show diff stats Hide diff stats

  1. 31  Core/Util/StringUtils.php
31  Core/Util/StringUtils.php
@@ -28,22 +28,33 @@ private function __construct() {}
28 28
      *
29 29
      * This method implements a constant-time algorithm to compare strings.
30 30
      *
31  
-     * @param string $str1 The first string
32  
-     * @param string $str2 The second string
  31
+     * @param string $knownString The string of known length to compare against
  32
+     * @param string $userInput   The string that the user can control
33 33
      *
34 34
      * @return Boolean true if the two strings are the same, false otherwise
35 35
      */
36  
-    public static function equals($str1, $str2)
  36
+    public static function equals($knownString, $userInput)
37 37
     {
38  
-        if (strlen($str1) !== $c = strlen($str2)) {
39  
-            return false;
40  
-        }
  38
+        // Prevent issues if string length is 0
  39
+        $knownString .= chr(0);
  40
+        $userInput .= chr(0);
  41
+
  42
+        $knownLen = strlen($knownString);
  43
+        $userLen = strlen($userInput);
  44
+
  45
+        // Set the result to the difference between the lengths
  46
+        $result = $knownLen - $userLen;
41 47
 
42  
-        $result = 0;
43  
-        for ($i = 0; $i < $c; $i++) {
44  
-            $result |= ord($str1[$i]) ^ ord($str2[$i]);
  48
+        // Note that we ALWAYS iterate over the user-supplied length
  49
+        // This is to prevent leaking length information
  50
+        for ($i = 0; $i < $userLen; $i++) {
  51
+            // Using % here is a trick to prevent notices
  52
+            // It's safe, since if the lengths are different
  53
+            // $result is already non-0
  54
+            $result |= (ord($knownString[$i % $knownLen]) ^ ord($userInput[$i]));
45 55
         }
46 56
 
47  
-        return 0 === $result;
  57
+        // They are only identical strings if $result is exactly 0...
  58
+        return $result === 0;
48 59
     }
49 60
 }

0 notes on commit 6ef999b

Please sign in to comment.
Something went wrong with that request. Please try again.