Merge pull request #6292 from kenjis/fix-Security-derandomize

fix: Security::derandomize() may cause hex2bin() error

Author: kenjis

system/Security/Security.php

@@ -21,6 +21,8 @@
 use Config\Cookie as CookieConfig;
 use Config\Security as SecurityConfig;
 use Config\Services;
+use ErrorException;
+use InvalidArgumentException;
 use LogicException;
 
 /**
@@ -278,8 +280,13 @@ public function verify(RequestInterface $request)
         }
 
         $postedToken = $this->getPostedToken($request);
-        $token       = ($postedToken !== null && $this->tokenRandomize)
-            ? $this->derandomize($postedToken) : $postedToken;
+
+        try {
+            $token = ($postedToken !== null && $this->tokenRandomize)
+                ? $this->derandomize($postedToken) : $postedToken;
+        } catch (InvalidArgumentException $e) {
+            $token = null;
+        }
 
         // Do the tokens match?
         if (! isset($token, $this->hash) || ! hash_equals($this->hash, $token)) {
@@ -359,13 +366,20 @@ protected function randomize(string $hash): string
 
     /**
      * Derandomize the token.
+     *
+     * @throws InvalidArgumentException "hex2bin(): Hexadecimal input string must have an even length"
      */
     protected function derandomize(string $token): string
     {
         $key   = substr($token, -static::CSRF_HASH_BYTES * 2);
         $value = substr($token, 0, static::CSRF_HASH_BYTES * 2);
 
-        return bin2hex(hex2bin($value) ^ hex2bin($key));
+        try {
+            return bin2hex(hex2bin($value) ^ hex2bin($key));
+        } catch (ErrorException $e) {
+            // "hex2bin(): Hexadecimal input string must have an even length"
+            throw new InvalidArgumentException($e->getMessage());
+        }
     }
 
     /**

tests/system/Security/SecurityCSRFSessionRandomizeTokenTest.php

@@ -141,6 +141,21 @@ public function testCSRFVerifyPostThrowsExceptionOnNoMatch()
         $security->verify($request);
     }
 
+    public function testCSRFVerifyPostInvalidToken()
+    {
+        $this->expectException(SecurityException::class);
+        $this->expectExceptionMessage('The action you requested is not allowed.');
+
+        $_SERVER['REQUEST_METHOD'] = 'POST';
+        $_POST['csrf_test_name']   = '!';
+
+        $request = new IncomingRequest(new MockAppConfig(), new URI('http://badurl.com'), null, new UserAgent());
+
+        $security = new Security(new MockAppConfig());
+
+        $security->verify($request);
+    }
+
     public function testCSRFVerifyPostReturnsSelfOnMatch()
     {
         $_SERVER['REQUEST_METHOD'] = 'POST';