dcodeIO/bcrypt.js

hashSync method silently replaces NaN salt rounds value with default value

p-brubaker opened this issue · 0 comments

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