dcodeIO / bcrypt.js

Optimized bcrypt in plain JavaScript with zero dependencies.
Other
3.47k stars 264 forks source link

Fix: Coerce `comp` to String Before Passing to `safeStringCompare` in bcrypt.hash #156

Open thehammadishaq opened 1 week ago

thehammadishaq commented 1 week ago

Summary

This pull request addresses an issue where the comp variable in bcrypt.hash might not be a string when passed to safeStringCompare. By coercing comp to a string, we avoid potential type-related issues and ensure proper comparison.

Steps to Reproduce

  1. Use bcrypt.hash with a non-string input.
  2. Observe the behavior.

Example:

const candidatePassword = 12345678; // Numerical password const hashedPassword = '$2a$10$Seh8RMKHzl3mfwwQYYUoBeUDLHMxEOK5QDs70aTQNpUpkK7P3qixe'; // Example hashed password

(async () => { try { const isMatch = await bcrypt.compare(candidatePassword, hashedPassword); console.log('Password Match:', isMatch); } catch (error) { console.error('Error comparing passwords:', error); } })();

Expected Behavior

bcrypt.compare should handle numerical candidate passwords gracefully, possibly by coercing them to strings internally, or should provide a clear error message indicating the type mismatch.

Actual Behavior

bcrypt.compare throws an "Illegal arguments: number, string" error.

Logs:

Error comparing passwords: Error: Illegal arguments: number, string at _async (path/to/bcrypt.js:286:46) at path/to/bcrypt.js:307:17 at new Promise () at bcrypt.compare (path/to/bcrypt.js:306:20) at path/to/your/code.js:10:38

Proposed Solution:

bcrypt.hash(s, hash.substr(0, 29), function(err, comp) { if (err) { callback(err); } else { callback(null, safeStringCompare(String(comp), hash)); } }, progressCallback);

This fix ensures that safeStringCompare receives strings, preventing potential type issues during password comparison.