Issue #2145881 by damiankloip, alexpott: Remove string casting from...

Issue #2145881 by damiankloip, alexpott: Remove string casting from Crypt::hmacBase64(). Throw exceptions instead.

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

@@ -71,21 +71,25 @@ public static function randomBytes($count) {
   /**
    * Calculates a base-64 encoded, URL-safe sha-256 hmac.
    *
-   * @param string $data
-   *   String to be validated with the hmac.
-   * @param string $key
-   *   A secret string key.
+   * @param mixed $data
+   *   Scalar value to be validated with the hmac.
+   * @param mixed $key
+   *   A secret key, this can be any scalar value.
    *
    * @return string
    *   A base-64 encoded sha-256 hmac, with + replaced with -, / with _ and
    *   any = padding characters removed.
    */
   public static function hmacBase64($data, $key) {
-    // Casting $data and $key to strings here is necessary to avoid empty string
+    // $data and $key being 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));
+    // function is used in security-critical contexts like token validation it
+    // is important that it never returns an empty string.
+    if (!is_scalar($data) || !is_scalar($key)) {
+      throw new \InvalidArgumentException('Both parameters passed to \Drupal\Component\Utility\Crypt::hmacBase64 must be scalar values.');
+    }
+
+    $hmac = base64_encode(hash_hmac('sha256', $data, $key, TRUE));
     // Modify the hmac so it's safe to use in URLs.
     return strtr($hmac, array('+' => '-', '/' => '_', '=' => ''));
   }

core/modules/system/lib/Drupal/system/Tests/Common/JavaScriptTest.php

@@ -37,6 +37,9 @@ public static function getInfo() {
 
   function setUp() {
     parent::setUp();
+    // There are dependencies in drupal_get_js() on the theme layer so we need
+    // to initialize it.
+    drupal_theme_initialize();
 
     // Disable preprocessing
     $config = \Drupal::config('system.performance');

core/modules/system/lib/Drupal/system/Tests/Common/RenderTest.php

@@ -29,6 +29,13 @@ public static function getInfo() {
     );
   }
 
+  function setUp() {
+    parent::setUp();
+    // There are dependencies in drupal_get_js() on the theme layer so we need
+    // to initialize it.
+    drupal_theme_initialize();
+  }
+
   /**
    * Tests the output drupal_render() for some elementary input values.
    */

core/tests/Drupal/Tests/Component/Utility/CryptTest.php

@@ -72,6 +72,21 @@ public function testHmacBase64($data, $key, $expected_hmac) {
     $this->assertEquals($expected_hmac, $hmac, 'The correct hmac was not calculated.');
   }
 
+  /**
+   * Tests the hmacBase64 method with invalid parameters.
+   *
+   * @param string $data
+   *   Data to hash.
+   * @param string $key
+   *   Key to use in hashing process.
+   *
+   * @dataProvider providerTestHmacBase64Invalid
+   * @expectedException InvalidArgumentException
+   */
+  public function testHmacBase64Invalid($data, $key) {
+    Crypt::hmacBase64($data, $key);
+  }
+
   /**
    * Provides data for self::testHashBase64().
    *
@@ -105,4 +120,39 @@ public function providerTestHmacBase64() {
     );
   }
 
+  /**
+   * Provides data for self::testHmacBase64().
+   *
+   * @return array Test data.
+   */
+  public function providerTestHmacBase64Invalid() {
+    return array(
+      array(new \stdClass(), new \stdClass()),
+      array(new \stdClass(), 'string'),
+      array(new \stdClass(), 1),
+      array(new \stdClass(), 0),
+      array(NULL, new \stdClass()),
+      array('string', new \stdClass()),
+      array(1, new \stdClass()),
+      array(0, new \stdClass()),
+      array(array(), array()),
+      array(array(), NULL),
+      array(array(), 'string'),
+      array(array(), 1),
+      array(array(), 0),
+      array(NULL, array()),
+      array(1, array()),
+      array(0, array()),
+      array('string', array()),
+      array(array(), NULL),
+      array(NULL, NULL),
+      array(NULL, 'string'),
+      array(NULL, 1),
+      array(NULL, 0),
+      array(1, NULL),
+      array(0, NULL),
+      array('string', NULL),
+    );
+  }
+
 }

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

@@ -97,33 +97,58 @@ public function testValidate() {
    *   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) {
+  public function testValidateParameterTypes($token, $value) {
     // 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));
+    $this->assertFalse($this->generator->validate($token, $value));
     restore_error_handler();
   }
 
   /**
-   * Provides data for the validate test.
+   * Provides data for testValidateParameterTypes.
    *
    * @return array
    *   An array of data used by the test.
    */
   public function providerTestValidateParameterTypes() {
+    return array(
+      array(array(), ''),
+      array(TRUE, 'foo'),
+      array(0, 'foo'),
+    );
+  }
+
+  /**
+   * Tests CsrfTokenGenerator::validate() with invalid parameter types.
+   *
+   * @param mixed $token
+   *   The token to be validated.
+   * @param mixed $value
+   *   (optional) An additional value to base the token on.
+   *
+   * @dataProvider providerTestInvalidParameterTypes
+   * @expectedException InvalidArgumentException
+   */
+  public function testInvalidParameterTypes($token, $value = '') {
+    $this->generator->validate($token, $value);
+  }
+
+  /**
+   * Provides data for testInvalidParameterTypes.
+   *
+   * @return array
+   *   An array of data used by the test.
+   */
+  public function providerTestInvalidParameterTypes() {
     return array(
       array(NULL, new \stdClass()),
       array(0, array()),
       array('', array()),
-      array(array()),
-      array(TRUE, 'foo'),
-      array(0, 'foo'),
+      array(array(), array()),
     );
   }