Browse Source

Use hmac for token hashes to avoid collisions

Marc Ypes 8 years ago
parent
commit
22b8fc4a24

+ 3 - 4
src/Controller/Component/SecurityComponent.php

@@ -314,9 +314,9 @@ class SecurityComponent extends Component
     {
         $token = $this->_validToken($controller);
         $hashParts = $this->_hashParts($controller);
-        $check = Security::hash(implode('', $hashParts), 'sha1');
+        $check = hash_hmac('sha1', implode('', $hashParts), Security::getSalt());
 
-        if ($token === $check) {
+        if (hash_equals($check, $token)) {
             return true;
         }
 
@@ -378,8 +378,7 @@ class SecurityComponent extends Component
         return [
             $controller->request->here(),
             serialize($fieldList),
-            $unlocked,
-            Security::getSalt()
+            $unlocked
         ];
     }
 

+ 2 - 3
src/View/Helper/SecureFieldTokenTrait.php

@@ -55,10 +55,9 @@ trait SecureFieldTokenTrait
         $hashParts = [
             $url,
             serialize($fields),
-            $unlocked,
-            Security::getSalt()
+            $unlocked
         ];
-        $fields = Security::hash(implode('', $hashParts), 'sha1');
+        $fields = hash_hmac('sha1', implode('', $hashParts), Security::getSalt());
 
         return [
             'fields' => urlencode($fields . ':' . $locked),

+ 25 - 21
tests/TestCase/Controller/Component/SecurityComponentTest.php

@@ -438,7 +438,7 @@ class SecurityComponentTest extends TestCase
         $event = new Event('Controller.startup', $this->Controller);
         $this->Security->startup($event);
 
-        $fields = '68730b0747d4889ec2766f9117405f9635f5fd5e%3AModel.valid';
+        $fields = 'b9b68375cbfc0476f175561c07b5e04d8908f3af%3AModel.valid';
         $unlocked = '';
         $debug = '';
 
@@ -619,7 +619,7 @@ class SecurityComponentTest extends TestCase
         $event = new Event('Controller.startup', $this->Controller);
         $this->Security->startup($event);
 
-        $fields = '8e26ef05379e5402c2c619f37ee91152333a0264%3A';
+        $fields = 'c0e09f0b7329e2e5873a7caaa687679e7ceec58b%3A';
         $unlocked = '';
         $debug = 'not used';
 
@@ -644,7 +644,7 @@ class SecurityComponentTest extends TestCase
         $event = new Event('Controller.startup', $this->Controller);
         $this->Security->startup($event);
 
-        $fields = '8e26ef05379e5402c2c619f37ee91152333a0264%3A';
+        $fields = 'c0e09f0b7329e2e5873a7caaa687679e7ceec58b%3A';
         $unlocked = '';
         $debug = urlencode(json_encode([
             'some-action',
@@ -677,7 +677,7 @@ class SecurityComponentTest extends TestCase
         $event = new Event('Controller.startup', $this->Controller);
         $this->Security->startup($event);
 
-        $fields = '4a221010dd7a23f7166cb10c38bc21d81341c387%3A';
+        $fields = 'b1af0affe0a36eb80f212082fd58fcda1e9e9451%3A';
         $unlocked = '';
         $debug = urlencode(json_encode([
             'some-action',
@@ -703,7 +703,7 @@ class SecurityComponentTest extends TestCase
         $event = new Event('Controller.startup', $this->Controller);
         $this->Security->startup($event);
 
-        $fields = 'a1c3724b7ba85e7022413611e30ba2c6181d5aba%3A';
+        $fields = '1463016f332f870ae1446a3e499ec39fa51647b1%3A';
         $unlocked = '';
         $debug = 'not used';
 
@@ -727,7 +727,7 @@ class SecurityComponentTest extends TestCase
         $event = new Event('Controller.startup', $this->Controller);
         $this->Security->startup($event);
 
-        $fields = 'b0914d06dfb04abf1fada53e16810e87d157950b%3A';
+        $fields = '1af7c2889ad58b14038d1552c6127a918bb5bcb7%3A';
         $unlocked = '';
         $debug = 'not used';
 
@@ -753,7 +753,7 @@ class SecurityComponentTest extends TestCase
         $event = new Event('Controller.startup', $this->Controller);
         $this->Security->startup($event);
 
-        $fields = 'b65c7463e44a61d8d2eaecce2c265b406c9c4742%3AAddresses.0.id%7CAddresses.1.id';
+        $fields = '4dda4baf4525f1593217aaa4f71fb83fbb947f52%3AAddresses.0.id%7CAddresses.1.id';
         $unlocked = '';
         $debug = 'not used';
 
@@ -787,7 +787,7 @@ class SecurityComponentTest extends TestCase
         $event = new Event('Controller.startup', $this->Controller);
         $this->Security->startup($event);
 
-        $fields = '8d8da68ba03b3d6e7e145b948abfe26741422169%3A';
+        $fields = 'c8d0d4939f3d84e0cccc2387ca39c722922eef2d%3A';
         $unlocked = '';
         $debug = 'not used';
 
@@ -812,7 +812,7 @@ class SecurityComponentTest extends TestCase
         $result = $this->validatePost();
         $this->assertTrue($result);
 
-        $fields = 'eae2adda1628b771a30cc133342d16220c6520fe%3A';
+        $fields = '176eb445b102cf7f85e38a7fc56f7e050143b664%3A';
         $this->Controller->request->data = [
             'User.password' => 'bar', 'User.name' => 'foo', 'User.is_valid' => '1',
             'Tag' => ['Tag' => [1]],
@@ -835,7 +835,7 @@ class SecurityComponentTest extends TestCase
     {
         $event = new Event('Controller.startup', $this->Controller);
         $this->Security->startup($event);
-        $fields = '68730b0747d4889ec2766f9117405f9635f5fd5e%3AModel.valid';
+        $fields = 'b9b68375cbfc0476f175561c07b5e04d8908f3af%3AModel.valid';
         $unlocked = '';
         $debug = 'not used';
 
@@ -847,7 +847,7 @@ class SecurityComponentTest extends TestCase
         $result = $this->validatePost();
         $this->assertTrue($result);
 
-        $fields = 'f63e4a69b2edd31f064e8e602a04dd59307cfe9c%3A';
+        $fields = 'e3704fbe647a57f5a99876335cb81ff424f46dc3%3A';
 
         $this->Controller->request->data = [
             'Model' => ['username' => '', 'password' => '', 'valid' => '0'],
@@ -879,7 +879,7 @@ class SecurityComponentTest extends TestCase
     {
         $event = new Event('Controller.startup', $this->Controller);
         $this->Security->startup($event);
-        $fields = '973a8939a68ac014cc6f7666cec9aa6268507350%3AModel.hidden%7CModel.other_hidden';
+        $fields = 'cbe3dc7c09e1ba2412316d1a1eff70d7f3791e1b%3AModel.hidden%7CModel.other_hidden';
         $unlocked = '';
         $debug = 'not used';
 
@@ -905,7 +905,7 @@ class SecurityComponentTest extends TestCase
         $event = new Event('Controller.startup', $this->Controller);
         $this->Security->config('disabledFields', ['Model.username', 'Model.password']);
         $this->Security->startup($event);
-        $fields = '1c59acfbca98bd870c11fb544d545cbf23215880%3AModel.hidden';
+        $fields = 'aab7b67fccb5a8fec8e5bc7f4585568e45cdecd1%3AModel.hidden';
         $unlocked = '';
         $debug = 'not used';
 
@@ -934,7 +934,9 @@ class SecurityComponentTest extends TestCase
         $this->Security->startup($event);
         $unlocked = 'Model.username';
         $fields = ['Model.hidden', 'Model.password'];
-        $fields = urlencode(Security::hash('/articles/index' . serialize($fields) . $unlocked . Security::salt()));
+        $fields = urlencode(
+            hash_hmac('sha1', '/articles/index' . serialize($fields) . $unlocked, Security::salt())
+        );
         $debug = 'not used';
 
         $this->Controller->request->data = [
@@ -1082,7 +1084,7 @@ class SecurityComponentTest extends TestCase
     {
         $event = new Event('Controller.startup', $this->Controller);
         $this->Security->startup($event);
-        $fields = '075ca6c26c38a09a78d871201df89faf52cbbeb8%3AModel.valid%7CModel2.valid%7CModel3.valid';
+        $fields = '1292df5b24f8c646cd93751bb9fb69d7972e69d0%3AModel.valid%7CModel2.valid%7CModel3.valid';
         $unlocked = '';
         $debug = 'not used';
 
@@ -1106,7 +1108,7 @@ class SecurityComponentTest extends TestCase
     {
         $event = new Event('Controller.startup', $this->Controller);
         $this->Security->startup($event);
-        $fields = '24a753fb62ef7839389987b58e3f7108f564e529%3AModel.0.hidden%7CModel.0.valid';
+        $fields = '203a106a01f120823baaab668112e30c4616e3a7%3AModel.0.hidden%7CModel.0.valid';
         $fields .= '%7CModel.1.hidden%7CModel.1.valid';
         $unlocked = '';
         $debug = 'not used';
@@ -1139,7 +1141,7 @@ class SecurityComponentTest extends TestCase
     {
         $event = new Event('Controller.startup', $this->Controller);
         $this->Security->startup($event);
-        $fields = '8f7d82bf7656cf068822d9bdab109ebed1be1825%3AAddress.0.id%7CAddress.0.primary%7C';
+        $fields = '2f086ffee2237435a13a88f38b91b0868c86ae40%3AAddress.0.id%7CAddress.0.primary%7C';
         $fields .= 'Address.1.id%7CAddress.1.primary';
         $unlocked = '';
         $debug = 'not used';
@@ -1188,7 +1190,9 @@ class SecurityComponentTest extends TestCase
         $this->Security->startup($event);
         $unlocked = '';
         $hashFields = ['TaxonomyData'];
-        $fields = urlencode(Security::hash('/articles/index' . serialize($hashFields) . $unlocked . Security::salt()));
+        $fields = urlencode(
+            hash_hmac('sha1', '/articles/index' . serialize($hashFields) . $unlocked, Security::salt())
+        );
         $debug = 'not used';
 
         $this->Controller->request->data = [
@@ -1280,7 +1284,7 @@ class SecurityComponentTest extends TestCase
         $event = new Event('Controller.startup', $this->Controller);
 
         $this->Security->startup($event);
-        $fields = '9da2b3fa2b5b8ac0bfbc1bbce145e58059629125%3An%3A0%3A%7B%7D';
+        $fields = '882b9a1226dd9fe8a2c8eed541dd3d059ba27d12%3An%3A0%3A%7B%7D';
         $unlocked = '';
         $debug = urlencode(json_encode([
             '/articles/index',
@@ -1319,7 +1323,7 @@ class SecurityComponentTest extends TestCase
     {
         $event = new Event('Controller.startup', $this->Controller);
         $this->Security->startup($event);
-        $fields = 'c2226a8879c3f4b513691295fc2519a29c44c8bb%3An%3A0%3A%7B%7D';
+        $fields = '14dc53654a1bfe184e921f0cb09a082003da1401%3An%3A0%3A%7B%7D';
         $unlocked = '';
         $debug = urlencode(json_encode([
             '/articles/index',
@@ -1368,7 +1372,7 @@ class SecurityComponentTest extends TestCase
         $event = new Event('Controller.startup', $this->Controller);
         $this->Security->startup($event);
 
-        $fields = 'b0914d06dfb04abf1fada53e16810e87d157950b%3A';
+        $fields = '1af7c2889ad58b14038d1552c6127a918bb5bcb7%3A';
         $unlocked = '';
         $debug = urlencode(json_encode([
             'another-url',

+ 16 - 31
tests/TestCase/View/Helper/FormHelperTest.php

@@ -1133,7 +1133,7 @@ class FormHelperTest extends TestCase
         $this->Form->request->params['_Token'] = 'foo';
 
         $result = $this->Form->secure(['anything']);
-        $this->assertRegExp('/540ac9c60d323c22bafe997b72c0790f39a8bdef/', $result);
+        $this->assertRegExp('/b9731869b9915e3dee6250db1a1fad464371fb94/', $result);
     }
 
     /**
@@ -1164,7 +1164,7 @@ class FormHelperTest extends TestCase
         $this->Form->request->params['_Token'] = 'testKey';
         $result = $this->Form->secure($fields);
 
-        $hash = Security::hash(serialize($fields) . Security::salt());
+        $hash = hash_hmac('sha1', serialize($fields), Security::salt());
         $hash .= ':' . 'Model.valid';
         $hash = urlencode($hash);
         $tokenDebug = urlencode(json_encode([
@@ -1212,7 +1212,7 @@ class FormHelperTest extends TestCase
         $this->Form->request->params['_Token'] = 'testKey';
         $result = $this->Form->secure($fields);
 
-        $hash = Security::hash(serialize($fields) . Security::salt());
+        $hash = hash_hmac('sha1', serialize($fields), Security::salt());
         $hash .= ':' . 'Model.valid';
         $hash = urlencode($hash);
         $expected = [
@@ -1406,6 +1406,7 @@ class FormHelperTest extends TestCase
         $result = $this->Form->secure($fields);
 
         $hash = '51e3b55a6edd82020b3f29c9ae200e14bbeb7ee5%3AModel.0.hidden%7CModel.0.valid';
+        $hash = '16e544e04f6d3007231e3e23f8f73427a53272d4%3AModel.0.hidden%7CModel.0.valid';
         $hash .= '%7CModel.1.hidden%7CModel.1.valid';
         $tokenDebug = urlencode(json_encode([
             '',
@@ -1606,8 +1607,7 @@ class FormHelperTest extends TestCase
         $this->Form->control('Addresses.1.primary', ['type' => 'checkbox']);
 
         $result = $this->Form->secure($this->Form->fields);
-
-        $hash = '8bd3911b07b507408b1a969b31ee90c47b7d387e%3AAddresses.0.id%7CAddresses.1.id';
+        $hash = '587942c6810603a6d5a07a394316dda455580227%3AAddresses.0.id%7CAddresses.1.id';
         $tokenDebug = urlencode(json_encode([
             '/articles/add',
             [
@@ -1704,7 +1704,7 @@ class FormHelperTest extends TestCase
         $this->Form->text('Addresses.1.phone');
 
         $result = $this->Form->secure($this->Form->fields);
-        $hash = '4fb10b46873df4ddd4ef5c3a19944a2f29b38991%3AAddresses.0.id%7CAddresses.1.id';
+        $hash = '8db4b5f1a912dfafd9c264964df7aa598ea322c0%3AAddresses.0.id%7CAddresses.1.id';
         $tokenDebug = urlencode(json_encode([
                 '/articles/add',
                 [
@@ -1782,7 +1782,7 @@ class FormHelperTest extends TestCase
 
         $result = $this->Form->secure($expected, ['data-foo' => 'bar']);
 
-        $hash = 'a303becbdd99cb42ca14a1cf7e63dfd48696a3c5%3AAddresses.id';
+        $hash = 'cdc8fa2dd2aa2804c12cd17279c39747f1c57354%3AAddresses.id';
         $tokenDebug = urlencode(json_encode([
                 '/articles/add',
                 [
@@ -1856,7 +1856,7 @@ class FormHelperTest extends TestCase
         $this->assertEquals($expected, $result);
         $result = $this->Form->secure($expected, ['data-foo' => 'bar', 'debugSecurity' => true]);
 
-        $hash = 'a303becbdd99cb42ca14a1cf7e63dfd48696a3c5%3AAddresses.id';
+        $hash = 'cdc8fa2dd2aa2804c12cd17279c39747f1c57354%3AAddresses.id';
         $tokenDebug = urlencode(json_encode([
             '/articles/add',
             [
@@ -1931,7 +1931,7 @@ class FormHelperTest extends TestCase
         Configure::write('debug', false);
         $result = $this->Form->secure($expected, ['data-foo' => 'bar', 'debugSecurity' => true]);
 
-        $hash = 'a303becbdd99cb42ca14a1cf7e63dfd48696a3c5%3AAddresses.id';
+        $hash = 'cdc8fa2dd2aa2804c12cd17279c39747f1c57354%3AAddresses.id';
         $expected = [
             'div' => ['style' => 'display:none;'],
             ['input' => [
@@ -1985,7 +1985,7 @@ class FormHelperTest extends TestCase
 
         $result = $this->Form->secure($expected, ['data-foo' => 'bar', 'debugSecurity' => false]);
 
-        $hash = 'a303becbdd99cb42ca14a1cf7e63dfd48696a3c5%3AAddresses.id';
+        $hash = 'cdc8fa2dd2aa2804c12cd17279c39747f1c57354%3AAddresses.id';
 
         $expected = [
             'div' => ['style' => 'display:none;'],
@@ -2479,7 +2479,7 @@ class FormHelperTest extends TestCase
     {
         $this->Form->request->params['_Token'] = ['key' => 'testKey'];
 
-        $expected = '0ff0c85cd70584d8fd18fa136846d22c66c21e2d%3A';
+        $expected = '8312b8faa7e74c6f36e05c8d188eda58b39fab20%3A';
         $this->Form->create($this->article, [
             'url' => ['controller' => 'articles', 'action' => 'view', 1, '?' => ['page' => 1]]
         ]);
@@ -2510,7 +2510,7 @@ class FormHelperTest extends TestCase
     {
         $this->Form->request->params['_Token'] = ['key' => 'testKey'];
 
-        $expected = 'ece0693fb1b19ca116133db1832ac29baaf41ce5%3A';
+        $expected = '93acdc2336947d62cf057a17275264c1fecc2443%3A';
         $this->Form->create($this->article, [
             'url' => [
                 'controller' => 'articles',
@@ -5577,7 +5577,7 @@ class FormHelperTest extends TestCase
         $this->assertEquals(['Model.multi_field'], $this->Form->fields);
 
         $result = $this->Form->secure($this->Form->fields);
-        $key = 'f7d573650a295b94e0938d32b323fde775e5f32b%3A';
+        $key = '3cecbba5b65c8792d963b0498c67741466e61d47%3A';
         $this->assertRegExp('/"' . $key . '"/', $result);
     }
 
@@ -7612,12 +7612,7 @@ class FormHelperTest extends TestCase
      */
     public function testPostLinkSecurityHash()
     {
-        $hash = Security::hash(
-            '/posts/delete/1' .
-            serialize(['id' => '1']) .
-            '' .
-            Security::salt()
-        );
+        $hash = hash_hmac('sha1', '/posts/delete/1' . serialize(['id' => '1']), Security::getSalt());
         $hash .= '%3Aid';
         $this->Form->request->params['_Token']['key'] = 'test';
 
@@ -7670,12 +7665,7 @@ class FormHelperTest extends TestCase
      */
     public function testPostLinkSecurityHashBlockMode()
     {
-        $hash = Security::hash(
-            '/posts/delete/1' .
-            serialize([]) .
-            '' .
-            Security::salt()
-        );
+        $hash = hash_hmac('sha1', '/posts/delete/1' . serialize([]), Security::getSalt());
         $hash .= '%3A';
         $this->Form->request->params['_Token']['key'] = 'test';
 
@@ -7699,12 +7689,7 @@ class FormHelperTest extends TestCase
     public function testPostLinkSecurityHashNoDebugMode()
     {
         Configure::write('debug', false);
-        $hash = Security::hash(
-            '/posts/delete/1' .
-            serialize(['id' => '1']) .
-            '' .
-            Security::salt()
-        );
+        $hash = hash_hmac('sha1', '/posts/delete/1' . serialize(['id' => '1']), Security::getSalt());
         $hash .= '%3Aid';
         $this->Form->request->params['_Token']['key'] = 'test';