Stricter validation for encrypted cookie

src/Utility/CookieCryptTrait.php

@@ -128,9 +128,15 @@ trait CookieCryptTrait
         }
         $this->_checkCipher($encrypt);
         $prefix = 'Q2FrZQ==.';
-        $value = base64_decode(substr($value, strlen($prefix)));
+        $prefixLength = strlen($prefix);
 
-        if ($value === '') {
+        if (strncmp($value, $prefix, $prefixLength) !== 0) {
+            return '';
+        }
+
+        $value = base64_decode(substr($value, $prefixLength), true);
+
+        if ($value === false ||$value === '') {
             return '';
         }
 
@@ -144,6 +150,10 @@ trait CookieCryptTrait
             $value = Security::decrypt($value, $key);
         }
 
+        if ($value === false) {
+            return '';
+        }
+
         return $this->_explode($value);
     }
 

tests/TestCase/Controller/Component/CookieComponentTest.php

@@ -657,23 +657,30 @@ class CookieComponentTest extends TestCase
     public function testReadingMalformedEncryptedCookies()
     {
         $this->Cookie->configKey('Encrypted_empty', 'encryption', 'aes');
-        $this->Cookie->configKey('Encrypted_too_short', 'encryption', 'aes');
+        $this->Cookie->configKey('Encrypted_wrong_prefix', 'encryption', 'aes');
         $this->Cookie->configKey('Encrypted_altered', 'encryption', 'aes');
+        $this->Cookie->configKey('Encrypted_invalid_chars', 'encryption', 'aes');
+
+        $encrypted = $this->_encrypt('secret data', 'aes');
 
         $this->Controller->request = $this->request->withCookieParams([
             'Encrypted_empty' => '',
-            'Encrypted_too_short' => 'Q2FrZQ',
-            'Encrypted_altered' => 'Q2FrZQ==.ModifiedBase64Data==',
+            'Encrypted_wrong_prefix' => substr_replace($encrypted, 'foo', 0, 3),
+            'Encrypted_altered' => str_replace('M', 'A', $encrypted),
+            'Encrypted_invalid_chars' => str_replace('M', 'M#', $encrypted),
         ]);
 
         $data = $this->Cookie->read('Encrypted_empty');
         $this->assertEquals('', $data);
 
-        $data = $this->Cookie->read('Encrypted_too_short');
+        $data = $this->Cookie->read('Encrypted_wrong_prefix');
         $this->assertEquals('', $data);
 
         $data = $this->Cookie->read('Encrypted_altered');
         $this->assertEquals('', $data);
+
+        $data = $this->Cookie->read('Encrypted_invalid_chars');
+        $this->assertEquals('', $data);
     }
 
     /**

tests/TestCase/Http/Middleware/EncryptedCookieMiddlewareTest.php

@@ -76,28 +76,23 @@ class EncryptedCookieMiddlewareTest extends TestCase
     /**
      * Test decoding malformed cookies
      *
+     * @dataProvider malformedCookies
+     * @param string $cookie
      * @return void
      */
-    public function testDecodeMalformedCookies()
+    public function testDecodeMalformedCookies($cookie)
     {
         $request = new ServerRequest(['url' => '/cookies/nom']);
-        $request = $request->withCookieParams([
-            'secret_empty' => '',
-            'secret_too_short' => 'Q2FrZQ',
-            'secret_altered' => 'Q2FrZQ==.ModifiedBase64Data==',
-        ]);
-        $this->assertNotEquals('decoded', $request->getCookie('decoded'));
+        $request = $request->withCookieParams(['secret' => $cookie]);
 
         $response = new Response();
         $next = function ($req, $res) {
-            $this->assertSame('', $req->getCookie('secret_empty'));
-            $this->assertSame('', $req->getCookie('secret_too_short'));
-            $this->assertSame('', $req->getCookie('secret_altered'));
+            $this->assertSame('', $req->getCookie('secret'));
 
             return $res;
         };
         $middleware = new EncryptedCookieMiddleware(
-            ['secret_empty', 'secret_too_short', 'secret_altered'],
+            ['secret'],
             $this->_getCookieEncryptionKey(),
             'aes'
         );
@@ -105,6 +100,23 @@ class EncryptedCookieMiddlewareTest extends TestCase
     }
 
     /**
+     * Data provider for malformed cookies.
+     *
+     * @return array
+     */
+    public function malformedCookies()
+    {
+        $encrypted = $this->_encrypt('secret data', 'aes');
+
+        return [
+            'empty' => [''],
+            'wrong prefix' => [substr_replace($encrypted, 'foo', 0, 3)],
+            'altered' => [str_replace('M', 'A', $encrypted)],
+            'invalid chars' => [str_replace('M', 'M#', $encrypted)],
+        ];
+    }
+
+    /**
      * Test encoding cookies in the set-cookie header.
      *
      * @return void