Fix CSRF token generation / validation

Operate on raw bytes instead of base64-encoded strings.
Issue was introduced in a977465

Signed-off-by: Leon Klingele <git@leonklingele.de>

diff --git a/lib/private/Security/CSRF/CsrfToken.php b/lib/private/Security/CSRF/CsrfToken.php
index dce9a83b727316520a6a463592abfb0c1c88d4ac..e9bdf5b52044ac46b4bb6130eb7ee5417289e9f7 100644 (file)
--- a/lib/private/Security/CSRF/CsrfToken.php
+++ b/lib/private/Security/CSRF/CsrfToken.php
@@ -51,8 +51,8 @@ class CsrfToken {
         */
        public function getEncryptedValue() {
                if($this->encryptedValue === '') {
-                       $sharedSecret = base64_encode(random_bytes(strlen($this->value)));
-                       $this->encryptedValue = base64_encode($this->value ^ $sharedSecret) . ':' . $sharedSecret;
+                       $sharedSecret = random_bytes(strlen($this->value));
+                       $this->encryptedValue = base64_encode($this->value ^ $sharedSecret) . ':' . base64_encode($sharedSecret);
                }
 
                return $this->encryptedValue;
@@ -71,6 +71,6 @@ class CsrfToken {
                }
                $obfuscatedToken = $token[0];
                $secret = $token[1];
-               return base64_decode($obfuscatedToken) ^ $secret;
+               return base64_decode($obfuscatedToken) ^ base64_decode($secret);
        }
 }

diff --git a/tests/lib/Security/CSRF/CsrfTokenManagerTest.php b/tests/lib/Security/CSRF/CsrfTokenManagerTest.php
index 6f7842fdfd9b61952119b44204ee598f12f8abaa..f9dd8127e5aa96ad8a1e13d76acfd6b9b5319c51 100644 (file)
--- a/tests/lib/Security/CSRF/CsrfTokenManagerTest.php
+++ b/tests/lib/Security/CSRF/CsrfTokenManagerTest.php
@@ -137,15 +137,19 @@ class CsrfTokenManagerTest extends \Test\TestCase {
        }
 
        public function testIsTokenValidWithValidToken() {
+               $a = 'abc';
+               $b = 'def';
+               $xorB64 = 'BQcF';
+               $tokenVal = sprintf('%s:%s', $xorB64, base64_encode($a));
                $this->storageInterface
                                ->expects($this->once())
                                ->method('hasToken')
                                ->willReturn(true);
-               $token = new \OC\Security\CSRF\CsrfToken('XlQhHjgWCgBXAEI0Khl+IQEiCXN2LUcDHAQTQAc1HQs=:qgkUlg8l3m8WnkOG4XM9Az33pAt1vSVMx4hcJFsxdqc=');
+               $token = new \OC\Security\CSRF\CsrfToken($tokenVal);
                $this->storageInterface
                                ->expects($this->once())
                                ->method('getToken')
-                               ->willReturn('/3JKTq2ldmzcDr1f5zDJ7Wt0lEgqqfKF');
+                               ->willReturn($b);
 
                $this->assertSame(true, $this->csrfTokenManager->isTokenValid($token));
        }

diff --git a/tests/lib/Security/CSRF/CsrfTokenTest.php b/tests/lib/Security/CSRF/CsrfTokenTest.php
index d19d1de916c93804b875a0b304e0a048b9fc5653..fbb92cd315ac4974296df8d9e06403fd04b17cb6 100644 (file)
--- a/tests/lib/Security/CSRF/CsrfTokenTest.php
+++ b/tests/lib/Security/CSRF/CsrfTokenTest.php
@@ -36,7 +36,11 @@ class CsrfTokenTest extends \Test\TestCase {
        }
 
        public function testGetDecryptedValue() {
-               $csrfToken = new \OC\Security\CSRF\CsrfToken('XlQhHjgWCgBXAEI0Khl+IQEiCXN2LUcDHAQTQAc1HQs=:qgkUlg8l3m8WnkOG4XM9Az33pAt1vSVMx4hcJFsxdqc=');
-               $this->assertSame('/3JKTq2ldmzcDr1f5zDJ7Wt0lEgqqfKF', $csrfToken->getDecryptedValue());
+               $a = 'abc';
+               $b = 'def';
+               $xorB64 = 'BQcF';
+               $tokenVal = sprintf('%s:%s', $xorB64, base64_encode($a));
+               $csrfToken = new \OC\Security\CSRF\CsrfToken($tokenVal);
+               $this->assertSame($b, $csrfToken->getDecryptedValue());
        }
 }