sha256 password before passing to bcrypt to avoid issues with 72 bytes truncation for passwords

[kossy0701]

More info: bcrypt-ruby/bcrypt-ruby#283

Reproduction:

BCrypt::Password.new(BCrypt::Password.create('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1')) == 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa2'
BCrypt::Password.new(BCrypt::Password.create('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1')) == 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa222333'
BCrypt::Password.new(BCrypt::Password.create('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1')) == 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa222333234234324'
All return true, so

Password 1: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1
Password 2: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa2

These two users can login to each other's accounts because brcypt caps hashing to the first 72 bytes.

hash = Devise::Encryptor.digest(Devise, 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1')
=> "$2a$13$XxwZStO7/NHTDjJsnGQhIOSb8ZO12PTL1/.Lze6OIT.qOAfBrqBHS"
Devise::Encryptor.compare(Devise, hash, 'password')
=> false
Devise::Encryptor.compare(Devise, hash, 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1')
=> true
Devise::Encryptor.compare(Devise, hash, 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa2')
=> true
As solution - hash password to sha256, so it always will be smaller, than 72 bytes. Added fallback for old passwords.

In this case we can reject heartcombo#5806

[greptile-apps]

Greptile Overview

Greptile Summary

This PR successfully addresses a critical security vulnerability in bcrypt where passwords longer than 72 bytes are silently truncated, allowing different passwords to authenticate as the same user. The fix applies SHA256 hashing to passwords before bcrypt, ensuring all passwords are reduced to a consistent 64-character hex string (well under the 72-byte limit).

Key changes:

• Applied Digest::SHA256.hexdigest before bcrypt in the digest method
• SHA256 hex output is always 64 characters, safely under bcrypt's 72-byte limit
• Added backward-compatible fallback in compare method to support existing password hashes
• Pepper is correctly applied before SHA256 hashing in both methods
• Added error handling for invalid bcrypt hashes with BCrypt::Errors::InvalidHash rescue
• Comprehensive test coverage including edge cases for 72+ byte passwords and legacy password support

The implementation is sound and maintains backward compatibility by trying the new SHA256 approach first, then falling back to the original direct bcrypt method for existing user passwords.

Confidence Score: 4/5

• safe to merge with one minor syntax issue that should be fixed
• addresses a legitimate security vulnerability with a well-established pattern (SHA256 before bcrypt), includes proper backward compatibility for existing passwords, and has comprehensive test coverage; only issue is a minor spacing error in test file
• test/encrytor_test.rb needs minor syntax correction on line 26

Important Files Changed

File Analysis

Filename	Score	Overview
lib/devise/encryptor.rb	4/5	implements SHA256 pre-hashing before bcrypt to fix 72-byte truncation vulnerability, includes backward-compatible fallback for existing passwords
test/encrytor_test.rb	4/5	comprehensive test coverage for new SHA256+bcrypt approach and backward compatibility, includes edge cases for long passwords and invalid hashes; minor syntax issue found

Sequence Diagram

sequenceDiagram
    participant User
    participant Devise
    participant Encryptor
    participant SHA256
    participant BCrypt
    
    Note over User,BCrypt: Password Registration (New)
    User->>Devise: Set password
    Devise->>Encryptor: digest(klass, password)
    Encryptor->>Encryptor: Apply pepper if present
    Encryptor->>SHA256: hexdigest(password+pepper)
    SHA256-->>Encryptor: 64-char hex string
    Encryptor->>BCrypt: create(hex_string)
    BCrypt-->>Encryptor: hashed_password
    Encryptor-->>Devise: encrypted_password
    
    Note over User,BCrypt: Password Validation (New Hash)
    User->>Devise: Login with password
    Devise->>Encryptor: compare(klass, hash, password)
    Encryptor->>Encryptor: Apply pepper if present
    Encryptor->>SHA256: hexdigest(password+pepper)
    SHA256-->>Encryptor: 64-char hex string
    Encryptor->>BCrypt: hash_secret(hex_string, salt)
    BCrypt-->>Encryptor: new_style_hash
    Encryptor->>Encryptor: secure_compare(new_style_hash, stored_hash)
    Encryptor-->>Devise: true/false
    
    Note over User,BCrypt: Password Validation (Old Hash - Fallback)
    User->>Devise: Login with password
    Devise->>Encryptor: compare(klass, hash, password)
    Encryptor->>Encryptor: Apply pepper if present
    Encryptor->>SHA256: hexdigest(password+pepper)
    SHA256-->>Encryptor: 64-char hex string
    Encryptor->>BCrypt: hash_secret(hex_string, salt)
    BCrypt-->>Encryptor: new_style_hash
    Encryptor->>Encryptor: secure_compare(new_style_hash, stored_hash)
    Note right of Encryptor: Returns false (old format)
    Encryptor->>BCrypt: hash_secret(password+pepper, salt)
    BCrypt-->>Encryptor: old_style_hash
    Encryptor->>Encryptor: secure_compare(old_style_hash, stored_hash)
    Encryptor-->>Devise: true/false

Loading

[greptile-apps]

_{2 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[kossy0701]

@greptile 修正しなきゃいけない行ってここで合ってますか？

[greptile-apps]

はい、その通りです。26行目が修正が必要な行で合っています。

この行では、古いbcryptのみのパスワード（SHA256ハッシュを使わない従来の方式）をテストするために、直接BCrypt::Password.createを呼び出しています。

新しい実装では、Devise::Encryptorが内部でSHA256ハッシュを適用してからbcryptを使用するように変更されているので、古い形式のハッシュとの後方互換性をテストする必要があります。

この行は意図的に古い方式（SHA256を使わずに直接bcryptを適用）でパスワードハッシュを生成することで、既存のユーザーのパスワードが新しいシステムでも正しく検証できることを確認するためのテストケースです。

[kossy0701]

@greptile

test/encrytor_test.rb needs minor syntax correction on line 26 ってコメントくれたけど、どんな変更が必要？

[greptile-apps]

26行目の構文エラーは、::BCryptの前にスペースが不要なことです。

Suggested change

	old_hashed_password =::BCrypt::Password.create(password_with_pepper, cost: Devise.stretches)
	old_hashed_password = ::BCrypt::Password.create(password_with_pepper, cost: Devise.stretches)

=の後の不要なスペースを削除するだけで修正できます。