diff options
| -rw-r--r-- | lib/private/appframework/http/request.php | 8 | ||||
| -rw-r--r-- | lib/private/security/crypto.php | 7 | ||||
| -rw-r--r-- | tests/lib/appframework/http/RequestTest.php | 95 |
3 files changed, 103 insertions, 7 deletions
diff --git a/lib/private/appframework/http/request.php b/lib/private/appframework/http/request.php index baf2f0c4745..43f01dfde3f 100644 --- a/lib/private/appframework/http/request.php +++ b/lib/private/appframework/http/request.php @@ -416,12 +416,10 @@ class Request implements \ArrayAccess, \Countable, IRequest { } // Check if the token is valid - if($token !== $this->items['requesttoken']) { - // Not valid - return false; - } else { - // Valid token + if(\OCP\Security\StringUtils::equals($token, $this->items['requesttoken'])) { return true; + } else { + return false; } } diff --git a/lib/private/security/crypto.php b/lib/private/security/crypto.php index 5a7073e950f..9bae1d6992c 100644 --- a/lib/private/security/crypto.php +++ b/lib/private/security/crypto.php @@ -27,7 +27,6 @@ use phpseclib\Crypt\AES; use phpseclib\Crypt\Hash; use OCP\Security\ICrypto; use OCP\Security\ISecureRandom; -use OCP\Security\StringUtils; use OCP\IConfig; /** @@ -50,6 +49,10 @@ class Crypto implements ICrypto { /** @var ISecureRandom */ private $random; + /** + * @param IConfig $config + * @param ISecureRandom $random + */ function __construct(IConfig $config, ISecureRandom $random) { $this->cipher = new AES(); $this->config = $config; @@ -119,7 +122,7 @@ class Crypto implements ICrypto { $this->cipher->setIV($iv); - if(!StringUtils::equals($this->calculateHMAC($parts[0].$parts[1], $password), $hmac)) { + if(!\OCP\Security\StringUtils::equals($this->calculateHMAC($parts[0].$parts[1], $password), $hmac)) { throw new \Exception('HMAC does not match.'); } diff --git a/tests/lib/appframework/http/RequestTest.php b/tests/lib/appframework/http/RequestTest.php index 6e86f3d7041..10a9e486c97 100644 --- a/tests/lib/appframework/http/RequestTest.php +++ b/tests/lib/appframework/http/RequestTest.php @@ -1156,4 +1156,99 @@ class RequestTest extends \Test\TestCase { $this->assertSame($expectedUri, $request->getRequestUri()); } + public function testPassesCSRFCheckWithGet() { + /** @var Request $request */ + $request = $this->getMockBuilder('\OC\AppFramework\Http\Request') + ->setMethods(['getScriptName']) + ->setConstructorArgs([ + [ + 'get' => [ + 'requesttoken' => 'MyStoredRequestToken', + ], + 'requesttoken' => 'MyStoredRequestToken', + ], + $this->secureRandom, + $this->config, + $this->stream + ]) + ->getMock(); + + $this->assertTrue($request->passesCSRFCheck()); + } + + public function testPassesCSRFCheckWithPost() { + /** @var Request $request */ + $request = $this->getMockBuilder('\OC\AppFramework\Http\Request') + ->setMethods(['getScriptName']) + ->setConstructorArgs([ + [ + 'post' => [ + 'requesttoken' => 'MyStoredRequestToken', + ], + 'requesttoken' => 'MyStoredRequestToken', + ], + $this->secureRandom, + $this->config, + $this->stream + ]) + ->getMock(); + + $this->assertTrue($request->passesCSRFCheck()); + } + + public function testPassesCSRFCheckWithHeader() { + /** @var Request $request */ + $request = $this->getMockBuilder('\OC\AppFramework\Http\Request') + ->setMethods(['getScriptName']) + ->setConstructorArgs([ + [ + 'server' => [ + 'HTTP_REQUESTTOKEN' => 'MyStoredRequestToken', + ], + 'requesttoken' => 'MyStoredRequestToken', + ], + $this->secureRandom, + $this->config, + $this->stream + ]) + ->getMock(); + + $this->assertTrue($request->passesCSRFCheck()); + } + + public function testPassesCSRFCheckWithInvalidToken() { + /** @var Request $request */ + $request = $this->getMockBuilder('\OC\AppFramework\Http\Request') + ->setMethods(['getScriptName']) + ->setConstructorArgs([ + [ + 'server' => [ + 'HTTP_REQUESTTOKEN' => 'MyInvalidSentToken', + ], + 'requesttoken' => 'MyStoredRequestToken', + ], + $this->secureRandom, + $this->config, + $this->stream + ]) + ->getMock(); + + $this->assertFalse($request->passesCSRFCheck()); + } + + public function testPassesCSRFCheckWithoutTokenFail() { + /** @var Request $request */ + $request = $this->getMockBuilder('\OC\AppFramework\Http\Request') + ->setMethods(['getScriptName']) + ->setConstructorArgs([ + [], + $this->secureRandom, + $this->config, + $this->stream + ]) + ->getMock(); + + $this->assertFalse($request->passesCSRFCheck()); + } + } |