CVE-2021-3944: Cross-Site Request Forgery (CSRF) (CWE-352)
value: bookstack is vulnerable to Cross-Site Request Forgery (CSRF)
CWE description: The web application does not, or can not, sufficiently verify whether a well-formed, valid, consistent request was intentionally provided by the user who submitted the request.
CVSS v3 base score: 6.8
Published: 2021-12-02T17:15Z
Repository: https://github.com/bookstackapp/bookstack
Language: PHP
Files changed: app/Http/Controllers/Auth/ConfirmEmailController.php, app/Http/Controllers/Auth/UserInviteController.php, resources/lang/en/auth.php, tests/Auth/AuthTest.php, tests/Auth/UserInviteTest.php
Commit date: 2021-11-15 10:50:28 +0000
Commit message: Prevented auto-login from direct email confirmation actions

Was done for convenience but could potentially be exploited by an
attacker using signing up via one of these routes, then forwarding
an email confirmation to another user so they unknowingly utilise
an account someone else controls.

Tweaks the flow of confirming email, and the user invite flow.

For #3050

Diff: diff --git a/app/Http/Controllers/Auth/ConfirmEmailController.php b/app/Http/Controllers/Auth/ConfirmEmailController.php
index 3e7d4a836..873d88475 100644
--- a/app/Http/Controllers/Auth/ConfirmEmailController.php
+++ b/app/Http/Controllers/Auth/ConfirmEmailController.php
@@ -77,13 +77,12 @@ class ConfirmEmailController extends Controller
         $user->email_confirmed = true;
         $user->save();
 
         $this->emailConfirmationService->deleteByUser($user);
         $this->showSuccessNotification(trans('auth.email_confirm_success'));
-        $this->loginService->login($user, auth()->getDefaultDriver());
 
-        return redirect('/');
+        return redirect('/login');
     }
 
     /**
      * Resend the confirmation email.
      */
diff --git a/app/Http/Controllers/Auth/UserInviteController.php b/app/Http/Controllers/Auth/UserInviteController.php
index b3ccf072a..df8262e22 100644
--- a/app/Http/Controllers/Auth/UserInviteController.php
+++ b/app/Http/Controllers/Auth/UserInviteController.php
@@ -1,10 +1,9 @@
 <?php
 
 namespace BookStack\Http\Controllers\Auth;
 
-use BookStack\Auth\Access\LoginService;
 use BookStack\Auth\Access\UserInviteService;
 use BookStack\Auth\UserRepo;
 use BookStack\Exceptions\UserTokenExpiredException;
 use BookStack\Exceptions\UserTokenNotFoundException;
 use BookStack\Http\Controllers\Controller;
@@ -14,23 +13,21 @@ use Illuminate\Http\Request;
 use Illuminate\Routing\Redirector;
 
 class UserInviteController extends Controller
 {
     protected $inviteService;
-    protected $loginService;
     protected $userRepo;
 
     /**
      * Create a new controller instance.
      */
-    public function __construct(UserInviteService $inviteService, LoginService $loginService, UserRepo $userRepo)
+    public function __construct(UserInviteService $inviteService, UserRepo $userRepo)
     {
         $this->middleware('guest');
         $this->middleware('guard:standard');
 
         $this->inviteService = $inviteService;
-        $this->loginService = $loginService;
         $this->userRepo = $userRepo;
     }
 
     /**
      * Show the page for the user to set the password for their account.
@@ -71,14 +68,13 @@ class UserInviteController extends Controller
         $user->password = bcrypt($request->get('password'));
         $user->email_confirmed = true;
         $user->save();
 
         $this->inviteService->deleteByUser($user);
-        $this->showSuccessNotification(trans('auth.user_invite_success', ['appName' => setting('app-name')]));
-        $this->loginService->login($user, auth()->getDefaultDriver());
+        $this->showSuccessNotification(trans('auth.user_invite_success_login', ['appName' => setting('app-name')]));
 
-        return redirect('/');
+        return redirect('/login');
     }
 
     /**
      * Check and validate the exception thrown when checking an invite token.
      *
diff --git a/resources/lang/en/auth.php b/resources/lang/en/auth.php
index ee0ba2d21..107585bf0 100644
--- a/resources/lang/en/auth.php
+++ b/resources/lang/en/auth.php
@@ -52,11 +52,11 @@ return [
     'email_confirm_subject' => 'Confirm your email on :appName',
     'email_confirm_greeting' => 'Thanks for joining :appName!',
     'email_confirm_text' => 'Please confirm your email address by clicking the button below:',
     'email_confirm_action' => 'Confirm Email',
     'email_confirm_send_error' => 'Email confirmation required but the system could not send the email. Contact the admin to ensure email is set up correctly.',
-    'email_confirm_success' => 'Your email has been confirmed!',
+    'email_confirm_success' => 'Your email has been confirmed! You should now be able to login using this email address.',
     'email_confirm_resent' => 'Confirmation email resent, Please check your inbox.',
 
     'email_not_confirmed' => 'Email Address Not Confirmed',
     'email_not_confirmed_text' => 'Your email address has not yet been confirmed.',
     'email_not_confirmed_click_link' => 'Please click the link in the email that was sent shortly after you registered.',
@@ -69,11 +69,11 @@ return [
     'user_invite_email_text' => 'Click the button below to set an account password and gain access:',
     'user_invite_email_action' => 'Set Account Password',
     'user_invite_page_welcome' => 'Welcome to :appName!',
     'user_invite_page_text' => 'To finalise your account and gain access you need to set a password which will be used to log-in to :appName on future visits.',
     'user_invite_page_confirm_button' => 'Confirm Password',
-    'user_invite_success' => 'Password set, you now have access to :appName!',
+    'user_invite_success_login' => 'Password set, you should now be able to login using your set password to access :appName!',
 
     // Multi-factor Authentication
     'mfa_setup' => 'Setup Multi-Factor Authentication',
     'mfa_setup_desc' => 'Setup multi-factor authentication as an extra layer of security for your user account.',
     'mfa_setup_configured' => 'Already configured',
diff --git a/tests/Auth/AuthTest.php b/tests/Auth/AuthTest.php
index 50f56bfb9..2c48131a8 100644
--- a/tests/Auth/AuthTest.php
+++ b/tests/Auth/AuthTest.php
@@ -129,12 +129,12 @@ class AuthTest extends TestCase
         Notification::assertSentTo($dbUser, ConfirmEmail::class, function ($notification, $channels) use ($emailConfirmation) {
             return $notification->token === $emailConfirmation->token;
         });
 
         // Check confirmation email confirmation activation.
-        $this->get('/register/confirm/' . $emailConfirmation->token)->assertRedirect('/');
-        $this->get('/')->assertSee($user->name);
+        $this->get('/register/confirm/' . $emailConfirmation->token)->assertRedirect('/login');
+        $this->get('/login')->assertSee('Your email has been confirmed! You should now be able to login using this email address.');
         $this->assertDatabaseMissing('email_confirmations', ['token' => $emailConfirmation->token]);
         $this->assertDatabaseHas('users', ['name' => $dbUser->name, 'email' => $dbUser->email, 'email_confirmed' => true]);
     }
 
     public function test_restricted_registration()
diff --git a/tests/Auth/UserInviteTest.php b/tests/Auth/UserInviteTest.php
index dcf9e23df..1e1235f33 100644
--- a/tests/Auth/UserInviteTest.php
+++ b/tests/Auth/UserInviteTest.php
@@ -50,11 +50,11 @@ class UserInviteTest extends TestCase
         $setPasswordPageResp->assertSee('Confirm Password');
 
         $setPasswordResp = $this->followingRedirects()->post('/register/invite/' . $token, [
             'password' => 'my test password',
         ]);
-        $setPasswordResp->assertSee('Password set, you now have access to BookStack!');
+        $setPasswordResp->assertSee('Password set, you should now be able to login using your set password to access BookStack!');
         $newPasswordValid = auth()->validate([
             'email'    => $user->email,
             'password' => 'my test password',
         ]);
         $this->assertTrue($newPasswordValid);

Vulnerable code:         $this->loginService->login($user, auth()->getDefaultDriver());
        return redirect('/');
use BookStack\Auth\Access\LoginService;
    protected $loginService;
    public function __construct(UserInviteService $inviteService, LoginService $loginService, UserRepo $userRepo)
        $this->loginService = $loginService;
        $this->showSuccessNotification(trans('auth.user_invite_success', ['appName' => setting('app-name')]));
        $this->loginService->login($user, auth()->getDefaultDriver());
        return redirect('/');
    'email_confirm_success' => 'Your email has been confirmed!',
    'user_invite_success' => 'Password set, you now have access to :appName!',
        $this->get('/register/confirm/' . $emailConfirmation->token)->assertRedirect('/');
        $this->get('/')->assertSee($user->name);
        $setPasswordResp->assertSee('Password set, you now have access to BookStack!');
Fixed code:         return redirect('/login');
    public function __construct(UserInviteService $inviteService, UserRepo $userRepo)
        $this->showSuccessNotification(trans('auth.user_invite_success_login', ['appName' => setting('app-name')]));
        return redirect('/login');
    'email_confirm_success' => 'Your email has been confirmed! You should now be able to login using this email address.',
    'user_invite_success_login' => 'Password set, you should now be able to login using your set password to access :appName!',
        $this->get('/register/confirm/' . $emailConfirmation->token)->assertRedirect('/login');
        $this->get('/login')->assertSee('Your email has been confirmed! You should now be able to login using this email address.');
        $setPasswordResp->assertSee('Password set, you should now be able to login using your set password to access BookStack!');