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

Security vulnerability: backup codes are stored as plain text #104

Open
evgenyneu opened this issue Oct 9, 2022 · 1 comment
Open

Security vulnerability: backup codes are stored as plain text #104

evgenyneu opened this issue Oct 9, 2022 · 1 comment

Comments

@evgenyneu
Copy link

I noticed a security vulnerability: the backup codes are stored as plain text in a database field. Specifically, authenticate_backup_code function checks if the list of stored backup codes includes the code entered by the user with backup_codes.include?(code) command:

def authenticate_backup_code(code)
        backup_codes_column_name = self.class.otp_backup_codes_column_name
        backup_codes = public_send(backup_codes_column_name)
        return false unless backup_codes.present? && backup_codes.include?(code)

The problem is, if the database leaks out, one can use the codes on the 2FA screen and bypass two factor authentication. A partial solution would be to encrypt the field that stores the backup codes with active record encryption or other methods. But it still makes the system vulnerable if both the database and the encryption key leaks out.

A conventional secure solution would be to generate a list of random backup codes but store only their hashes in the database. One of popular password hashing methods is bcrypt. Here is how to generate a code and its hash:

require 'bcrypt'

backup_code = SecureRandom.alphanumeric(BACKUP_CODE_LENGTH)
backup_code_hash = BCrypt::Password.create(backup_code)

And here is how to check if the code entered by the user corresponds to the generated hash:

BCrypt::Password.new(backup_code_hash) == backup_code_from_the_user
@guilleiguaran
Copy link
Member

Thanks for the bug report @evgenyneu, do you want to contribute a pull request with the solution?

Otherwise, I'll take some time to fix this.

I don't mind adding bcrypt as a dependency since Rails had is as soft-dependency as well.

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