From 2c8402aa170d445c549f0409325730e780eb4764 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 5 Mar 2018 15:01:02 +0100 Subject: [PATCH 1/2] Make \OC\Security\CSRF strict Signed-off-by: Roeland Jago Douma --- lib/private/Security/CSRF/CsrfToken.php | 11 ++++++----- lib/private/Security/CSRF/CsrfTokenGenerator.php | 3 ++- lib/private/Security/CSRF/CsrfTokenManager.php | 9 +++++---- .../Security/CSRF/TokenStorage/SessionStorage.php | 7 ++++--- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/lib/private/Security/CSRF/CsrfToken.php b/lib/private/Security/CSRF/CsrfToken.php index d9e27ff80e3..643e58e1d53 100644 --- a/lib/private/Security/CSRF/CsrfToken.php +++ b/lib/private/Security/CSRF/CsrfToken.php @@ -1,4 +1,5 @@ value = $value; } @@ -50,9 +51,9 @@ class CsrfToken { * * @return string */ - public function getEncryptedValue() { + public function getEncryptedValue(): string { if($this->encryptedValue === '') { - $sharedSecret = random_bytes(strlen($this->value)); + $sharedSecret = random_bytes(\strlen($this->value)); $this->encryptedValue = base64_encode($this->value ^ $sharedSecret) . ':' . base64_encode($sharedSecret); } @@ -65,9 +66,9 @@ class CsrfToken { * * @return string */ - public function getDecryptedValue() { + public function getDecryptedValue(): string { $token = explode(':', $this->value); - if (count($token) !== 2) { + if (\count($token) !== 2) { return ''; } $obfuscatedToken = $token[0]; diff --git a/lib/private/Security/CSRF/CsrfTokenGenerator.php b/lib/private/Security/CSRF/CsrfTokenGenerator.php index 85207956e1a..be628ea176c 100644 --- a/lib/private/Security/CSRF/CsrfTokenGenerator.php +++ b/lib/private/Security/CSRF/CsrfTokenGenerator.php @@ -1,4 +1,5 @@ random->generate($length); } } diff --git a/lib/private/Security/CSRF/CsrfTokenManager.php b/lib/private/Security/CSRF/CsrfTokenManager.php index b43ca3d3679..deacd1f512c 100644 --- a/lib/private/Security/CSRF/CsrfTokenManager.php +++ b/lib/private/Security/CSRF/CsrfTokenManager.php @@ -1,4 +1,5 @@ csrfToken)) { + public function getToken(): CsrfToken { + if(!\is_null($this->csrfToken)) { return $this->csrfToken; } @@ -73,7 +74,7 @@ class CsrfTokenManager { * * @return CsrfToken */ - public function refreshToken() { + public function refreshToken(): CsrfToken { $value = $this->tokenGenerator->generateToken(); $this->sessionStorage->setToken($value); $this->csrfToken = new CsrfToken($value); @@ -94,7 +95,7 @@ class CsrfTokenManager { * @param CsrfToken $token * @return bool */ - public function isTokenValid(CsrfToken $token) { + public function isTokenValid(CsrfToken $token): bool { if(!$this->sessionStorage->hasToken()) { return false; } diff --git a/lib/private/Security/CSRF/TokenStorage/SessionStorage.php b/lib/private/Security/CSRF/TokenStorage/SessionStorage.php index 946330b0c8c..b35e148c7ce 100644 --- a/lib/private/Security/CSRF/TokenStorage/SessionStorage.php +++ b/lib/private/Security/CSRF/TokenStorage/SessionStorage.php @@ -1,4 +1,5 @@ session->get('requesttoken'); if(empty($token)) { throw new \Exception('Session does not contain a requesttoken'); @@ -68,7 +69,7 @@ class SessionStorage { * * @param string $value */ - public function setToken($value) { + public function setToken(string $value) { $this->session->set('requesttoken', $value); } @@ -83,7 +84,7 @@ class SessionStorage { * * @return bool */ - public function hasToken() { + public function hasToken(): bool { return $this->session->exists('requesttoken'); } } From d1791864303f97011318fc41d2cc0914e2aa1c11 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 5 Mar 2018 16:14:46 +0100 Subject: [PATCH 2/2] Remove testcase Since a token now always requires a string we don't need to test for null Signed-off-by: Roeland Jago Douma --- tests/lib/AppFramework/Http/RequestTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/lib/AppFramework/Http/RequestTest.php b/tests/lib/AppFramework/Http/RequestTest.php index c6b9719b32a..a715eaa9599 100644 --- a/tests/lib/AppFramework/Http/RequestTest.php +++ b/tests/lib/AppFramework/Http/RequestTest.php @@ -1846,7 +1846,6 @@ class RequestTest extends \Test\TestCase { return [ ['InvalidSentToken'], ['InvalidSentToken:InvalidSecret'], - [null], [''], ]; }