Provisional OAuth, OpenID token check timing attack patches

2 files changed, 54 insertions, 1 deletions

diff --git a/extlib/Auth/OpenID/Association.php b/extlib/Auth/OpenID/Association.php
index d1ac1ed9b..7fdf399a3 100644
--- a/extlib/Auth/OpenID/Association.php
+++ b/extlib/Auth/OpenID/Association.php
@@ -374,7 +374,42 @@ class Auth_OpenID_Association {
         }
 
         $calculated_sig = $this->getMessageSignature($message);
-        return $calculated_sig == $sig;
+
+        return $this->constantTimeCompare($calculated_sig, $sig);
+    }
+
+    /**
+     * String comparison function which will complete in a constant time
+     * for strings of any given matching length, to help prevent an attacker
+     * from distinguishing how much of a signature token they have guessed
+     * correctly.
+     *
+     * For this usage, it's assumed that the length of the string is known,
+     * so we may safely short-circuit on mismatched lengths which will be known
+     * to be invalid by the attacker.
+     *
+     * http://lists.openid.net/pipermail/openid-security/2010-July/001156.html
+     * http://rdist.root.org/2010/01/07/timing-independent-array-comparison/
+     */
+    private function constantTimeCompare($a, $b)
+    {
+        $len = strlen($a);
+        if (strlen($b) !== $len) {
+            // Short-circuit on length mismatch; attackers will already know
+            // the correct target length so this is safe.
+            return false;
+        }
+        if ($len == 0) {
+            // 0-length valid input shouldn't really happen. :)
+            return true;
+        }
+        $result = 0;
+        for ($i = 0; $i < strlen($a); $i++) {
+            // We use scary bitwise operations to avoid logical short-circuits
+            // in lower-level code.
+            $result |= ord($a{$i}) ^ ord($b{$i});
+        }
+        return ($result == 0);
     }
 }
 
diff --git a/extlib/OAuth.php b/extlib/OAuth.php
index 648627b57..04984d5fa 100644
--- a/extlib/OAuth.php
+++ b/extlib/OAuth.php
@@ -54,6 +54,24 @@ class OAuthSignatureMethod {/*{{{*/
   public function check_signature(&$request, $consumer, $token, $signature) {
     $built = $this->build_signature($request, $consumer, $token);
     return $built == $signature;
+
+    // Check for zero length, although unlikely here
+    if (strlen($built) == 0 || strlen($signature) == 0) {
+      return false;
+    }
+
+    if (strlen($built) != strlen($signature)) {
+      return false;
+    }
+
+    $result = 0;
+
+    // Avoid a timing leak with a (hopefully) time insensitive compare
+    for ($i = 0; $i < strlen($signature); $i++) {
+      $result |= ord($built{$i}) ^ ord($signature{$i});
+    }
+
+    return $result == 0;
   }
 }/*}}}*/