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

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

Open
thehammadishaq opened this issue Jun 26, 2024 · 0 comments

Comments

@thehammadishaq
Copy link

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.

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

1 participant