heapsource / active_model_otp

Adds methods to set and authenticate against one time passwords (Two-Factor Authentication). Inspired in AM::SecurePassword
MIT License
773 stars 81 forks source link

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

Open evgenyneu opened 2 years ago

evgenyneu commented 2 years ago

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 commented 1 year ago

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.