search

jaredhanson / passport-totp

TOTP authentication strategy for Passport and Node.js.
MIT License
151 stars 47 forks source link

TOTP key is accessible to attacker in example app #9

Open robmaceachern opened 7 years ago

robmaceachern commented 7 years ago

I believe there is a bug in the /setup route of the example app that allows an attacker to acquire the TOTP key on a user account that has 2 factor authentication enabled (if they know the user's login credentials).

Steps to reproduce:

Set up

  1. Start the example app web server
  2. Go to http://localhost:3000/ in browser
  3. Log in with bob:secret
  4. Go to "Account"
  5. Scan QR code with Authenticator app
  6. Go to "Account"
  7. Submit code from Authenticator app
  8. Successfully log in to "Account"

As attacker

  1. Delete "bob@example.com" entry from Google Authenticator
  2. Open different web browser than the one used in set up.
  3. Go to http://localhost:3000
  4. Log in with bob:secret
  5. Go to http://localhost:3000/setup
  6. Scan QR code with Authenticator app
  7. Go to "Account"
  8. Submit code from Authenticator app
  9. Successfully log in to "Account"

If a user account already has two factor enabled, the key should only be exposed when the user has authenticated with the second factor.

unknowndomain commented 7 years ago

Part of the issue is that the pattern this example follows is unlike any other TOTP you'll see implemented in the wild...

Normally the secret is only exposed during setup, then you provide a sample code back from Google Authenticator to the website to prove you saved it properly, then you never again see the code.

I've implemented this a different way, first I generate a random secret, display the QR code, and a field to collect the sample code back from the user, with a hidden input field with the secret in it.

Then the user submits the sample code, and I use notp.totp (which is what passport-totp uses) to verify the code was valid within a delta of 1 (Math.abs), if it was I save and 2FA is enabled, if not then it generate a new secret and starts again.

There is a vulnerability here that the user could modify their secret, so I intend to store this in the database with a flag for 'active' and regenerate the secret every time they load the page unless they've activated through the above process.

This would avoid this security problem.