bug symfony#46309 [Security] Fix division by zero (tvlooy)

chalasr · chalasr · commit 5584221cde29 · 2022-05-11T18:57:29.000+02:00
This PR was merged into the 5.4 branch. Discussion ---------- [Security] Fix division by zero | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | | License | MIT | Doc PR | Given: CSRF token abc.def.ghi was returned When: I change the value of this token in my browser to abc..ghi Then: the key becomes '' and the xor that is called in denormalize results in a division by zero and http 500 Commits ------- 5028662 Fix division by zero
diff --git a/src/Symfony/Component/Security/Csrf/CsrfTokenManager.php b/src/Symfony/Component/Security/Csrf/CsrfTokenManager.php
@@ -134,6 +134,9 @@ private function derandomize(string $value): string
             return $value;
         }
         $key = base64_decode(strtr($parts[1], '-_', '+/'));
+        if ('' === $key || false === $key) {
+            return $value;
+        }
         $value = base64_decode(strtr($parts[2], '-_', '+/'));
 
         return $this->xor($value, $key);
diff --git a/src/Symfony/Component/Security/Csrf/Tests/CsrfTokenManagerTest.php b/src/Symfony/Component/Security/Csrf/Tests/CsrfTokenManagerTest.php
@@ -193,6 +193,26 @@ public function testNonExistingTokenIsNotValid($namespace, $manager, $storage)
         $this->assertFalse($manager->isTokenValid(new CsrfToken('token_id', 'FOOBAR')));
     }
 
+    public function testTokenShouldNotTriggerDivisionByZero()
+    {
+        [$generator, $storage] = $this->getGeneratorAndStorage();
+        $manager = new CsrfTokenManager($generator, $storage);
+
+        // Scenario: the token that was returned is abc.def.ghi, and gets modified in the browser to abc..ghi
+
+        $storage->expects($this->once())
+            ->method('hasToken')
+            ->with('https-token_id')
+            ->willReturn(true);
+
+        $storage->expects($this->once())
+            ->method('getToken')
+            ->with('https-token_id')
+            ->willReturn('def');
+
+        $this->assertFalse($manager->isTokenValid(new CsrfToken('token_id', 'abc..ghi')));
+    }
+
     /**
      * @dataProvider getManagerGeneratorAndStorage
      */

Original file line number	Diff line number	Diff line change
`@@ -134,6 +134,9 @@ private function derandomize(string $value): string`
`134`	`134`	`return $value;`
`135`	`135`	`}`
`136`	`136`	`$key = base64_decode(strtr($parts[1], '-_', '+/'));`
	`137`	`+ if ('' === $key \|\| false === $key) {`
	`138`	`+ return $value;`
	`139`	`+ }`
`137`	`140`	`$value = base64_decode(strtr($parts[2], '-_', '+/'));`
`138`	`141`
`139`	`142`	`return $this->xor($value, $key);`