Skip to content

Commit

Permalink
Merge pull request from GHSA-5hm8-vh6r-2cjq
Browse files Browse the repository at this point in the history
fix: CSRF protection bypass vulnerability
  • Loading branch information
MGatner authored Aug 7, 2022
2 parents c95a28e + 58686b3 commit 342a368
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 4 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ access for a mobile application that you build.

Usage of Shield requires the following:

- A [CodeIgniter 4](https://github.com/codeigniter4/CodeIgniter4/)-based project
- A [CodeIgniter 4.2.3+](https://github.com/codeigniter4/CodeIgniter4/) based project
- [Composer](https://getcomposer.org/) for package management
- PHP 7.4.3+

Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
},
"require-dev": {
"codeigniter4/devkit": "^1.0",
"codeigniter4/framework": "^4.1",
"codeigniter4/framework": "^4.2.3",
"mockery/mockery": "^1.0"
},
"provide": {
Expand Down
6 changes: 6 additions & 0 deletions docs/install.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@

These instructions assume that you have already [installed the CodeIgniter 4 app starter](https://codeigniter.com/user_guide/installation/installing_composer.html) as the basis for your new project, set up your `.env` file, and created a database that you can access via the Spark CLI script.

> **Note**
> CodeIgniter Shield requires Codeigniter v4.2.3 or later.
> **Note**
> You must set ``Config\Security::$csrfProtection`` to `'session'` (or set `security.csrfProtection = session` in your `.env` file) for security reasons, if you use Session Authenticator.
Installation is done through [Composer](https://getcomposer.org). The example assumes you have it installed globally.
If you have it installed as a phar, or othewise you will need to adjust the way you call composer itself.

Expand Down
27 changes: 26 additions & 1 deletion src/Authentication/Authenticators/Session.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,14 @@
use CodeIgniter\Shield\Entities\UserIdentity;
use CodeIgniter\Shield\Exceptions\InvalidArgumentException;
use CodeIgniter\Shield\Exceptions\LogicException;
use CodeIgniter\Shield\Exceptions\SecurityException;
use CodeIgniter\Shield\Models\LoginModel;
use CodeIgniter\Shield\Models\RememberModel;
use CodeIgniter\Shield\Models\UserIdentityModel;
use CodeIgniter\Shield\Models\UserModel;
use CodeIgniter\Shield\Result;
use Config\Security;
use Config\Services;
use stdClass;

class Session implements AuthenticatorInterface
Expand Down Expand Up @@ -72,6 +75,25 @@ public function __construct(UserModel $provider)
$this->loginModel = model(LoginModel::class);
$this->rememberModel = model(RememberModel::class);
$this->userIdentityModel = model(UserIdentityModel::class);

$this->checkSecurityConfig();
}

/**
* Checks less secure Configuration.
*/
private function checkSecurityConfig(): void
{
/** @var Security $securityConfig */
$securityConfig = config('Security');

if ($securityConfig->csrfProtection === 'cookie') {
throw new SecurityException(
'Config\Security::$csrfProtection is set to \'cookie\'.'
. ' Same-site attackers may bypass the CSRF protection.'
. ' Please set it to \'session\'.'
);
}
}

/**
Expand Down Expand Up @@ -567,7 +589,10 @@ public function startLogin(User $user): void

// Regenerate the session ID to help protect against session fixation
if (ENVIRONMENT !== 'testing') {
session()->regenerate();
session()->regenerate(true);

// Regenerate CSRF token even if `security.regenerate = false`.
Services::security()->generateHash();
}

// Let the session know we're logged in
Expand Down
9 changes: 9 additions & 0 deletions src/Exceptions/SecurityException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?php

namespace CodeIgniter\Shield\Exceptions;

use RuntimeException;

class SecurityException extends RuntimeException
{
}
1 change: 0 additions & 1 deletion tests/Controllers/RegisterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ protected function setUp(): void
parent::setUp();

helper('auth');
Factories::reset();

// Add auth routes
$routes = service('routes');
Expand Down
5 changes: 5 additions & 0 deletions tests/_support/TestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,10 @@ protected function setUp(): void
$config = config('Auth');
$config->actions = ['login' => null, 'register' => null];
Factories::injectMock('config', 'Auth', $config);

// Set Config\Security::$csrfProtection to 'session'
$config = config('Security');
$config->csrfProtection = 'session';
Factories::injectMock('config', 'Security', $config);
}
}

0 comments on commit 342a368

Please sign in to comment.