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.

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

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

Open
skeets23 opened this issue Feb 2, 2023 · 4 comments
Open

Comments

@skeets23
Copy link

skeets23 commented Feb 2, 2023

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
Copy link
Contributor

mfn commented Feb 2, 2023

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
Copy link
Author

skeets23 commented Feb 2, 2023

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
Copy link
Contributor

mfn commented Feb 2, 2023

🤷🏼

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

@skeets23
Copy link
Author

skeets23 commented Feb 2, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants