Skip to content
Permalink
Browse files Browse the repository at this point in the history
fix: [security] Further fixes to the bruteforce handling
- resolved a potential failure of the subsystem when the MySQL and the webserver time settings are diverged
  - as reported by Dawid Czarnecki
- several tightenings of the checks to avoid potential foul play
  • Loading branch information
iglocska committed Feb 10, 2020
1 parent 9400b8b commit 934c828
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 6 deletions.
3 changes: 2 additions & 1 deletion app/Controller/UsersController.php
Expand Up @@ -1066,6 +1066,7 @@ public function login()
$this->Bruteforce = ClassRegistry::init('Bruteforce');
if (!empty($this->request->data['User']['email'])) {
if ($this->Bruteforce->isBlacklisted($_SERVER['REMOTE_ADDR'], $this->request->data['User']['email'])) {
$expire = Configure::check('SecureAuth.expire') ? Configure::read('SecureAuth.expire') : 300;
throw new ForbiddenException('You have reached the maximum number of login attempts. Please wait ' . Configure::read('SecureAuth.expire') . ' seconds and try again.');
}
}
Expand Down Expand Up @@ -1116,7 +1117,7 @@ public function login()
$this->Session->delete('Message.auth');
}
// don't display "invalid user" before first login attempt
if ($this->request->is('post')) {
if ($this->request->is('post') || $this->request->is('put')) {
$this->Flash->error(__('Invalid username or password, try again'));
if (isset($this->request->data['User']['email'])) {
$this->Bruteforce->insert($_SERVER['REMOTE_ADDR'], $this->request->data['User']['email']);
Expand Down
13 changes: 8 additions & 5 deletions app/Model/Bruteforce.php
Expand Up @@ -9,17 +9,19 @@ public function insert($ip, $username)
{
$this->Log = ClassRegistry::init('Log');
$this->Log->create();
$expire = time() + Configure::read('SecureAuth.expire');
$expire = Configure::check('SecureAuth.expire') ? Configure::read('SecureAuth.expire') : 300;
$amount = Configure::check('SecureAuth.amount') ? Configure::read('SecureAuth.amount') : 5;
$expire = time() + $expire;
$expire = date('Y-m-d H:i:s', $expire);
$bruteforceEntry = array(
'ip' => $ip,
'username' => $username,
'username' => trim(strtolower($username)),
'expire' => $expire
);
$this->save($bruteforceEntry);
$title = 'Failed login attempt using username ' . $username . ' from IP: ' . $_SERVER['REMOTE_ADDR'] . '.';
if ($this->isBlacklisted($ip, $username)) {
$title .= 'This has tripped the bruteforce protection after ' . Configure::read('SecureAuth.amount') . ' failed attempts. The user is now blacklisted for ' . Configure::read('SecureAuth.expire') . ' seconds.';
$title .= 'This has tripped the bruteforce protection after ' . $amount . ' failed attempts. The user is now blacklisted for ' . $expire . ' seconds.';
}
$log = array(
'org' => 'SYSTEM',
Expand All @@ -36,10 +38,11 @@ public function clean()
{
$dataSourceConfig = ConnectionManager::getDataSource('default')->config;
$dataSource = $dataSourceConfig['datasource'];
$expire = date('Y-m-d H:i:s', time());
if ($dataSource == 'Database/Mysql') {
$sql = 'DELETE FROM bruteforces WHERE `expire` <= NOW();';
$sql = 'DELETE FROM bruteforces WHERE `expire` <= "' . $expire . '";';
} elseif ($dataSource == 'Database/Postgres') {
$sql = 'DELETE FROM bruteforces WHERE expire <= NOW();';
$sql = 'DELETE FROM bruteforces WHERE expire <= "' . $expire . '";';
}
$this->query($sql);
}
Expand Down

0 comments on commit 934c828

Please sign in to comment.