Skip to content

Coerce comp to String Before Passing to safeStringCompare in bcrypt.hash #157

Closed as not planned
@thehammadishaq

Description

@thehammadishaq

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions