From 5d64056e89ba95f3d4fe704fdaa6450b8f152158 Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Sun, 9 Jun 2019 00:25:26 +0200 Subject: [PATCH] Implement middleware for CSRF token verification This fixes a rather large oversight in Flarum's codebase, which was that we had no explicit CSRF protection using the traditional token approach. The JS frontend was actually sending these tokens, but the backend did not require them. --- src/Admin/AdminServiceProvider.php | 1 + src/Api/ApiServiceProvider.php | 1 + src/Forum/ForumServiceProvider.php | 1 + .../Middleware/AuthenticateWithHeader.php | 2 + src/Http/Middleware/CheckCsrfToken.php | 46 +++++++++++++++++++ 5 files changed, 51 insertions(+) create mode 100644 src/Http/Middleware/CheckCsrfToken.php diff --git a/src/Admin/AdminServiceProvider.php b/src/Admin/AdminServiceProvider.php index 5e61729bd..493464ba9 100644 --- a/src/Admin/AdminServiceProvider.php +++ b/src/Admin/AdminServiceProvider.php @@ -61,6 +61,7 @@ class AdminServiceProvider extends AbstractServiceProvider $pipe->pipe($app->make(HttpMiddleware\StartSession::class)); $pipe->pipe($app->make(HttpMiddleware\RememberFromCookie::class)); $pipe->pipe($app->make(HttpMiddleware\AuthenticateWithSession::class)); + $pipe->pipe($app->make(HttpMiddleware\CheckCsrfToken::class)); $pipe->pipe($app->make(HttpMiddleware\SetLocale::class)); $pipe->pipe($app->make(Middleware\RequireAdministrateAbility::class)); diff --git a/src/Api/ApiServiceProvider.php b/src/Api/ApiServiceProvider.php index eb077259c..dd8eb6cd9 100644 --- a/src/Api/ApiServiceProvider.php +++ b/src/Api/ApiServiceProvider.php @@ -57,6 +57,7 @@ class ApiServiceProvider extends AbstractServiceProvider $pipe->pipe($app->make(HttpMiddleware\RememberFromCookie::class)); $pipe->pipe($app->make(HttpMiddleware\AuthenticateWithSession::class)); $pipe->pipe($app->make(HttpMiddleware\AuthenticateWithHeader::class)); + $pipe->pipe($app->make(HttpMiddleware\CheckCsrfToken::class)); $pipe->pipe($app->make(HttpMiddleware\SetLocale::class)); event(new ConfigureMiddleware($pipe, 'api')); diff --git a/src/Forum/ForumServiceProvider.php b/src/Forum/ForumServiceProvider.php index cdb4e0e48..56edb7132 100644 --- a/src/Forum/ForumServiceProvider.php +++ b/src/Forum/ForumServiceProvider.php @@ -68,6 +68,7 @@ class ForumServiceProvider extends AbstractServiceProvider $pipe->pipe($app->make(HttpMiddleware\StartSession::class)); $pipe->pipe($app->make(HttpMiddleware\RememberFromCookie::class)); $pipe->pipe($app->make(HttpMiddleware\AuthenticateWithSession::class)); + $pipe->pipe($app->make(HttpMiddleware\CheckCsrfToken::class)); $pipe->pipe($app->make(HttpMiddleware\SetLocale::class)); $pipe->pipe($app->make(HttpMiddleware\ShareErrorsFromSession::class)); diff --git a/src/Http/Middleware/AuthenticateWithHeader.php b/src/Http/Middleware/AuthenticateWithHeader.php index 787470c90..87f5bb51b 100644 --- a/src/Http/Middleware/AuthenticateWithHeader.php +++ b/src/Http/Middleware/AuthenticateWithHeader.php @@ -44,6 +44,8 @@ class AuthenticateWithHeader implements Middleware $token->touch(); $actor = $token->user; + + $request = $request->withAttribute('bypassCsrfToken', true); } if (isset($actor)) { diff --git a/src/Http/Middleware/CheckCsrfToken.php b/src/Http/Middleware/CheckCsrfToken.php new file mode 100644 index 000000000..d3efe18fd --- /dev/null +++ b/src/Http/Middleware/CheckCsrfToken.php @@ -0,0 +1,46 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Flarum\Http\Middleware; + +use Flarum\Http\Exception\TokenMismatchException; +use Psr\Http\Message\ResponseInterface as Response; +use Psr\Http\Message\ServerRequestInterface as Request; +use Psr\Http\Server\MiddlewareInterface as Middleware; +use Psr\Http\Server\RequestHandlerInterface as Handler; + +class CheckCsrfToken implements Middleware +{ + public function process(Request $request, Handler $handler): Response + { + if (in_array($request->getMethod(), ['GET', 'HEAD', 'OPTIONS'])) { + return $handler->handle($request); + } + + if ($request->getAttribute('bypassCsrfToken', false)) { + return $handler->handle($request); + } + + if ($this->tokensMatch($request)) { + return $handler->handle($request); + } + + throw new TokenMismatchException('CSRF token did not match'); + } + + private function tokensMatch(Request $request): bool + { + $expected = (string) $request->getAttribute('session')->token(); + $provided = $request->getHeaderLine('X-CSRF-Token'); // TODO: Use form field, if provided + + return hash_equals($expected, $provided); + } +}