Issue #2141041 by Heine, klausi, David_Rothstein, amateescu, tim.plunkett, :...

Issue #2141041 by Heine, klausi, David_Rothstein, amateescu, tim.plunkett, : CsrfTokenGenerator::validate() should do an identical compare. (CORE-SA-2013-003 follow-up)

core/lib/Drupal/Component/Utility/Crypt.php

@@ -81,7 +81,11 @@ public static function randomBytes($count) {
    *   any = padding characters removed.
    */
   public static function hmacBase64($data, $key) {
-    $hmac = base64_encode(hash_hmac('sha256', $data, $key, TRUE));
+    // Casting $data and $key to strings here is necessary to avoid empty string
+    // results of the hash function if they are not scalar values. As this
+    // function is used in security-critical contexts like token validation it is
+    // important that it never returns an empty string.
+    $hmac = base64_encode(hash_hmac('sha256', (string) $data, (string) $key, TRUE));
     // Modify the hmac so it's safe to use in URLs.
     return strtr($hmac, array('+' => '-', '/' => '_', '=' => ''));
   }

core/lib/Drupal/Core/Access/CsrfTokenGenerator.php

@@ -84,7 +84,7 @@ public function get($value = '') {
    *   is TRUE, the return value will always be TRUE for anonymous users.
    */
   public function validate($token, $value = '', $skip_anonymous = FALSE) {
-    return ($skip_anonymous && $this->currentUser->isAnonymous()) || ($token == $this->get($value));
+    return ($skip_anonymous && $this->currentUser->isAnonymous()) || ($token === $this->get($value));
   }
 
 }

core/tests/Drupal/Tests/Core/Access/CsrfTokenGeneratorTest.php

@@ -90,6 +90,43 @@ public function testValidate() {
     $this->assertFalse($this->generator->validate($token, 'foo', TRUE));
   }
 
+  /**
+   * Tests CsrfTokenGenerator::validate() with different parameter types.
+   *
+   * @param mixed $token
+   *   The token to be validated.
+   * @param mixed $value
+   *   (optional) An additional value to base the token on.
+   * @param mixed $expected
+   *   (optional) The expected result of validate(). Defaults to FALSE.
+   *
+   * @dataProvider providerTestValidateParameterTypes
+   */
+  public function testValidateParameterTypes($token, $value = '', $expected = FALSE) {
+    // The following check might throw PHP fatals and notices, so we disable
+    // error assertions.
+    set_error_handler(function () {return TRUE;});
+    $this->assertSame($expected, $this->generator->validate($token, $value));
+    restore_error_handler();
+  }
+
+  /**
+   * Provides data for the validate test.
+   *
+   * @return array
+   *   An array of data used by the test.
+   */
+  public function providerTestValidateParameterTypes() {
+    return array(
+      array(NULL, new \stdClass()),
+      array(0, array()),
+      array('', array()),
+      array(array()),
+      array(TRUE, 'foo'),
+      array(0, 'foo'),
+    );
+  }
+
 }
 
 }