From 17d08cc0a4f6fd131a429b6c5d8fa49a1e20e151 Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Sat, 25 Jun 2016 18:21:52 +0200 Subject: [PATCH] fixed crash on non-string input to CSRF token fixes #11822 also adding proper unit tests for validate CSRF token. --- framework/CHANGELOG.md | 1 + framework/web/Request.php | 4 ++ tests/framework/web/RequestTest.php | 93 +++++++++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+) diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index 11682cd..2de211d 100644 --- a/framework/CHANGELOG.md +++ b/framework/CHANGELOG.md @@ -41,6 +41,7 @@ Yii Framework 2 Change Log - Bug #11672: Fixed `yii\validators\NumberValidator` erroring when value is an object without `__toString()` method (SamMousa) - Bug #11561: Fixed DI container throwing exceptions for optional dependencies (SamMousa) - Enh #11168: `yii\helpers\BaseHtml` now uses abstracted `booleanInput()` and `activeBooleanInput()` methods to render `radio()`, `checkbox()`, `activeRadio()` and `activeCheckbox()` (cesarnicola) +- Bug #11822: Fixed exception on non-string value provided as CSRF token (cebe) 2.0.8 April 28, 2016 -------------------- diff --git a/framework/web/Request.php b/framework/web/Request.php index 9ead092..ce503a7 100644 --- a/framework/web/Request.php +++ b/framework/web/Request.php @@ -1403,6 +1403,10 @@ class Request extends \yii\base\Request */ private function validateCsrfTokenInternal($token, $trueToken) { + if (!is_string($token)) { + return false; + } + $token = base64_decode(str_replace('.', '+', $token)); $n = StringHelper::byteLength($token); if ($n <= static::CSRF_MASK_LENGTH) { diff --git a/tests/framework/web/RequestTest.php b/tests/framework/web/RequestTest.php index 8ee890a..b5a59b6 100644 --- a/tests/framework/web/RequestTest.php +++ b/tests/framework/web/RequestTest.php @@ -88,7 +88,100 @@ class RequestTest extends TestCase $token = $request->getCsrfToken(); + // accept any value if CSRF validation is disabled + $request->enableCsrfValidation = false; $this->assertTrue($request->validateCsrfToken($token)); + $this->assertTrue($request->validateCsrfToken($token . 'a')); + $this->assertTrue($request->validateCsrfToken([])); + $this->assertTrue($request->validateCsrfToken([$token])); + $this->assertTrue($request->validateCsrfToken(0)); + $this->assertTrue($request->validateCsrfToken(null)); + + // enable validation + $request->enableCsrfValidation = true; + + // accept any value on GET request + foreach(['GET', 'HEAD', 'OPTIONS'] as $method) { + $_POST[$request->methodParam] = $method; + $this->assertTrue($request->validateCsrfToken($token)); + $this->assertTrue($request->validateCsrfToken($token . 'a')); + $this->assertTrue($request->validateCsrfToken([])); + $this->assertTrue($request->validateCsrfToken([$token])); + $this->assertTrue($request->validateCsrfToken(0)); + $this->assertTrue($request->validateCsrfToken(null)); + } + + // only accept valid token on POST + foreach(['POST', 'PUT', 'DELETE'] as $method) { + $_POST[$request->methodParam] = $method; + $this->assertTrue($request->validateCsrfToken($token)); + $this->assertFalse($request->validateCsrfToken($token . 'a')); + $this->assertFalse($request->validateCsrfToken([])); + $this->assertFalse($request->validateCsrfToken([$token])); + $this->assertFalse($request->validateCsrfToken(0)); + $this->assertFalse($request->validateCsrfToken(null)); + } + } + + /** + * test CSRF token validation by POST param + */ + public function testCsrfTokenPost() + { + $this->mockWebApplication(); + + $request = new Request(); + $request->enableCsrfCookie = false; + + $token = $request->getCsrfToken(); + + // accept no value on GET request + foreach(['GET', 'HEAD', 'OPTIONS'] as $method) { + $_POST[$request->methodParam] = $method; + $this->assertTrue($request->validateCsrfToken()); + } + + // only accept valid token on POST + foreach(['POST', 'PUT', 'DELETE'] as $method) { + $_POST[$request->methodParam] = $method; + $request->setBodyParams([]); + $this->assertFalse($request->validateCsrfToken()); + $request->setBodyParams([$request->csrfParam => $token]); + $this->assertTrue($request->validateCsrfToken()); + } + + } + + /** + * test CSRF token validation by POST param + */ + public function testCsrfTokenHeader() + { + $this->mockWebApplication(); + + $request = new Request(); + $request->enableCsrfCookie = false; + + $token = $request->getCsrfToken(); + + // accept no value on GET request + foreach(['GET', 'HEAD', 'OPTIONS'] as $method) { + $_POST[$request->methodParam] = $method; + $this->assertTrue($request->validateCsrfToken()); + } + + // only accept valid token on POST + foreach(['POST', 'PUT', 'DELETE'] as $method) { + $_POST[$request->methodParam] = $method; + $request->setBodyParams([]); + //$request->headers->remove(Request::CSRF_HEADER); + unset($_SERVER['HTTP_' . str_replace('-', '_', strtoupper(Request::CSRF_HEADER))]); + $this->assertFalse($request->validateCsrfToken()); + //$request->headers->add(Request::CSRF_HEADER, $token); + $_SERVER['HTTP_' . str_replace('-', '_', strtoupper(Request::CSRF_HEADER))] = $token; + $this->assertTrue($request->validateCsrfToken()); + } + } public function testResolve()