antonioribeiro / google2fa-laravel

A One Time Password Authentication package, compatible with Google Authenticator for Laravel
MIT License
928 stars 180 forks source link

Huge security flaw allowing bypass of 2FA completely in some scenarios #180

Open skeets23 opened 1 year ago

skeets23 commented 1 year ago

In Authenticator.php:

protected function canPassWithoutCheckingOTP()
    {
        return
            !$this->isEnabled() ||
            $this->noUserIsAuthenticated() ||
            !$this->isActivated() ||
            $this->twoFactorAuthStillValid();
    }

So, if no user is authenticated, they can "pass without checking OTP".

This should be the opposite, if there's no user authenticated and they are trying to visit a page that requires 2FA to access, doesn't that mean we should BLOCK them instead of allowing them to by-pass 2FA?

If a secondary session is used by the project to determine if they are "logged in", this means they can just delete their Laravel session cookie, and by-pass 2FA completely.

Needless to say, this is a major issue.

mfn commented 1 year ago

Not an expert, my understanding:

This package deals with 2fa only, you need to authenticate the user first by using the auth middleware. This goes hand in hand.

skeets23 commented 1 year ago

Yes, I have a separate authentication middleware. I guess the problem arises when the project's definition of "logged in" doesn't match this package. In a project with support for a legacy system where the user needs to be "logged in" either by the Laravel session or a vanilla PHP session, this means they can simply delete their Laravel session cookie and avoid bypass 2FA completely.

mfn commented 1 year ago

🤷🏼

FWIF, I use 2FA together with legacy PHP session support (shared with another framework 😅) and it "works for me".

skeets23 commented 1 year ago

This vulnerability will only affect you if you can delete the Laravel session cookie and stay logged in.