Skip to content

Commit 659c33f

Browse files
committed
Fixes a bug that allows a user to bypass 2FA authentication requirements
This bug was reported to us by a user (@ferrypterodactyl#1704) on Discord on Monday, November 7th, 2016. It was disclosed that it was possible to bypass the 2FA checkpoint by clicking outside of the modal which would prompt the modal to close, but not submit the form. The user could then press the login button which would trigger an error. Due to this error being triggered the authentication attempt was not cancelled. On the next page load the application recognized the user as logged in and continued on to the panel. At no time was it possible to login without using the correct email address and password. As a result of this bug we have re-factored the Authentication code for logins to address the persistent session. Previously accounts were manually logged back out on 2FA failure. However, as this bug demonstrated, causing a fatal error in the code would prevent the logout code from firing, thus preserving their session state. This commit modifies the code to use a non-persistent login to handle 2FA checking. In order for the session to be saved the application must complete all portions of the login without any errors, at which point the user is persistently authenticated using Auth::login(). This resolves the ability to cause an exception and bypass 2FA verification.
1 parent e77b984 commit 659c33f

File tree

2 files changed

+10
-9
lines changed

2 files changed

+10
-9
lines changed

app/Http/Controllers/Auth/LoginController.php

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ public function login(Request $request)
103103
}
104104

105105
// Is the email & password valid?
106-
if (!Auth::attempt([
106+
if (!Auth::once([
107107
'email' => $request->input('email'),
108108
'password' => $request->input('password')
109109
], $request->has('remember'))) {
@@ -116,14 +116,10 @@ public function login(Request $request)
116116

117117
}
118118

119-
$G2FA = new Google2FA();
120-
$user = User::select('use_totp', 'totp_secret')->where('email', $request->input('email'))->first();
121-
122119
// Verify TOTP Token was Valid
123-
if($user->use_totp === 1) {
124-
if(!$G2FA->verifyKey($user->totp_secret, $request->input('totp_token'))) {
125-
126-
Auth::logout();
120+
if(Auth::user()->use_totp === 1) {
121+
$G2FA = new Google2FA();
122+
if(is_null($request->input('totp_token')) || !$G2FA->verifyKey(Auth::user()->totp_secret, $request->input('totp_token'))) {
127123

128124
if (!$lockedOut) {
129125
$this->incrementLoginAttempts($request);
@@ -135,6 +131,8 @@ public function login(Request $request)
135131
}
136132
}
137133

134+
// Successfully Authenticated.
135+
Auth::login(Auth::user(), $request->has('remember'));
138136
return $this->sendLoginResponse($request);
139137

140138
}

resources/views/auth/login.blade.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,10 @@
125125
}
126126
}).done(function (data) {
127127
if (typeof data.id !== 'undefined') {
128-
$('#openTOTP').modal('show');
128+
$('#openTOTP').modal({
129+
backdrop: 'static',
130+
keyboard: false
131+
});
129132
$('#openTOTP').on('shown.bs.modal', function() {
130133
$('#totp_token').focus();
131134
});

0 commit comments

Comments
 (0)