Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[testing] Fixed bug #683 Failed Login Counter Reset Triggered by Found User in develop branch. #684

Closed
wants to merge 3 commits into from
Closed

[testing] Fixed bug #683 Failed Login Counter Reset Triggered by Found User in develop branch. #684

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d09bdbd
Fixed bug #683 Failed Login Counter Reset Triggered by Found User in …
Jul 22, 2015
4f46a8e
Renamed last-failed-login session var to last-user-not-found to enhan…
justincdotme Jul 27, 2015
288aaef
Extracted user-not-found increment code to LoginModel::incrementUserN…
justincdotme Jul 27, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 32 additions & 13 deletions application/model/LoginModel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,26 +83,20 @@ private static function validateAndGetUser($user_name, $user_password)
// brute force attack mitigation: use session failed login count and last failed login for not found users.
// block login attempt if somebody has already failed 3 times and the last login attempt is less than 30sec ago
// (limits user searches in database)
if (Session::get('failed-login-count') >= 3 AND (Session::get('last-failed-login') > (time() - 30))) {
if (Session::get('user-not-found-count') >= 3 AND (Session::get('last-user-not-found') > (time() - 30))) {
Session::add('feedback_negative', Text::get('FEEDBACK_LOGIN_FAILED_3_TIMES'));
return false;
}

// get all data of that user (to later check if password and password_hash fit)
$result = UserModel::getUserDataByUsername($user_name);

// check if that user exists. We don't give back a cause in the feedback to avoid giving an attacker details.
// brute force attack mitigation: reset failed login counter because of found user
if ($result){
Session::set('failed-login-count', 0);
Session::set('last-failed-login', '');
} else {
// brute force attack mitigation: set session failed login count and last failed login for users not found
Session::set('failed-login-count', Session::get('failed-login-count') + 1);
Session::set('last-failed-login', time());
Session::add('feedback_negative', Text::get('FEEDBACK_LOGIN_FAILED_3_TIMES'));
return false;
}
if (!$result){
//Increment the user not found count. Helps mitigate user enumeration.
self::incrementUserNotFoundCounter();
return false;
}

// block login attempt if somebody has already failed 3 times and the last login attempt is less than 30sec ago
if (($result->user_failed_logins >= 3) AND ($result->user_last_failed_login > (time() - 30))) {
Expand All @@ -124,9 +118,34 @@ private static function validateAndGetUser($user_name, $user_password)
return false;
}

//Reset the user not found counter.
self::resetUserNotFoundCounter();
return $result;
}

/**
* Increment the user-not-found-count by 1.
* Add timestamp to last-user-not-found.
*
*/
private static function incrementUserNotFoundCounter()
{
// Username enumeration prevention: set session failed login count and last failed login for users not found
Session::set('user-not-found-count', Session::get('user-not-found-count') + 1);
Session::set('last-user-not-found', time());
}

/**
* Reset the user-not-found-count to 0.
* Reset the last-user-not-found to an empty string.
*
*/
private static function resetUserNotFoundCounter()
{
Session::set('user-not-found-count', 0);
Session::set('last-user-not-found', '');
}

/**
* performs the login via cookie (for DEFAULT user account, FACEBOOK-accounts are handled differently)
* TODO add throttling here ?
Expand Down