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

hashSync method silently replaces NaN salt rounds value with default value #126

Open
p-brubaker opened this issue Nov 13, 2021 · 0 comments

Comments

@p-brubaker
Copy link

p-brubaker commented Nov 13, 2021

First, the illegitimate value NaN for the salt rounds parameter makes its way through the hashSync method because it has type 'number'.

Then the genSaltSync method is called and passed the salt rounds parameter NaN, where it is silently replaced by the default value, 10, because NaN is falsy.

This could be a problem if, for example, the value NaN was the result of a missing environment variable that had been coerced to a number from a string. I would expect there to be a warning, if not an error, since an error is thrown when 'undefined' or some other illegitimate value for the salt rounds parameter is passed to the hashSync method.

The end result is the number of salt rounds actually being used may be completely different from what was intended, but no warning is raised.

Brief example code:

const bcrypt = require("bcryptjs");

const missingEnvVariable = "undefined";

const rounds = Number(missingEnvVariable);
console.log("rounds: ", rounds);

const exampleNewPasswordHash = bcrypt.hashSync("password", rounds);

console.log("hash: ", exampleNewPasswordHash);

console output:

rounds:  NaN
hash:  $2a$10$0Dni/5gUiFddXJ5m0bk1peaWU.Me5HmPEipvy5Vp7cdDR00ZndfAG

// node version: 16.9.1
// OS: Ubuntu 20.04.1 LTS (fossa-bulbasaur X54) 64-bit

@p-brubaker p-brubaker changed the title hashSync method silently replaces NaN salt rounds value with 10 hashSync method silently replaces NaN salt rounds value with default value Nov 13, 2021
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