Browse Source

Merge pull request #14680 from cakephp/issue-14675

Fix missing typechecks in SecurityComponent and CsrfProtectionMiddleware
Mark Story 5 years ago
parent
commit
6546f1b698

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

@@ -360,6 +360,9 @@ class SecurityComponent extends Component
         if (!isset($check['_Token']['fields'])) {
             throw new AuthSecurityException(sprintf($message, '_Token.fields'));
         }
+        if (!is_string($check['_Token']['fields'])) {
+            throw new AuthSecurityException("'_Token.fields' was invalid.");
+        }
         if (!isset($check['_Token']['unlocked'])) {
             throw new AuthSecurityException(sprintf($message, '_Token.unlocked'));
         }

+ 1 - 1
src/Http/Middleware/CsrfProtectionMiddleware.php

@@ -105,7 +105,7 @@ class CsrfProtectionMiddleware
         $cookies = $request->getCookieParams();
         $cookieData = Hash::get($cookies, $this->_config['cookieName']);
 
-        if (strlen($cookieData) > 0) {
+        if (is_string($cookieData) && strlen($cookieData) > 0) {
             $params = $request->getAttribute('params');
             $params['_csrfToken'] = $cookieData;
             $request = $request->withAttribute('params', $params);

+ 26 - 2
tests/TestCase/Controller/Component/SecurityComponentTest.php

@@ -567,14 +567,14 @@ class SecurityComponentTest extends TestCase
     }
 
     /**
-     * testValidatePostFormHacking method
+     * testValidatePostFormTampering method
      *
      * Test that validatePost fails if any of its required fields are missing.
      *
      * @return void
      * @triggers Controller.startup $this->Controller
      */
-    public function testValidatePostFormHacking()
+    public function testValidatePostFormTampering()
     {
         $event = new Event('Controller.startup', $this->Controller);
         $this->Security->startup($event);
@@ -1140,6 +1140,30 @@ class SecurityComponentTest extends TestCase
     }
 
     /**
+     * Test that invalid types cause failures.
+     *
+     * @return void
+     */
+    public function testValidatePostFailArrayData()
+    {
+        $event = new Event('Controller.startup', $this->Controller);
+        $this->Security->startup($event);
+        $this->Controller->request = $this->Controller->request->withParsedBody([
+            'Model' => [
+                'username' => 'mark',
+                'password' => 'sekret',
+            ],
+            '_Token' => [
+                'fields' => [],
+                'unlocked' => '',
+            ],
+        ]);
+        Configure::write('debug', false);
+        $result = $this->validatePost('SecurityException', "'_Token.fields' was invalid.");
+        $this->assertFalse($result);
+    }
+
+    /**
      * testValidateHiddenMultipleModel method
      *
      * @return void

+ 21 - 0
tests/TestCase/Http/Middleware/CsrfProtectionMiddlewareTest.php

@@ -232,6 +232,27 @@ class CsrfProtectionMiddlewareTest extends TestCase
     }
 
     /**
+     * Test that request data works with the various http methods.
+     *
+     * @return void
+     */
+    public function testInvalidTokenNonStringData()
+    {
+        $this->expectException(\Cake\Http\Exception\InvalidCsrfTokenException::class);
+        $request = new ServerRequest([
+            'environment' => [
+                'REQUEST_METHOD' => 'POST',
+            ],
+            'post' => ['_csrfToken' => ['nope']],
+            'cookies' => ['csrfToken' => ['nope']],
+        ]);
+        $response = new Response();
+
+        $middleware = new CsrfProtectionMiddleware();
+        $middleware($request, $response, $this->_getNextClosure());
+    }
+
+    /**
      * Test that missing header and cookie fails
      *
      * @dataProvider httpMethodProvider