dcodeIO / bcrypt.js

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

Illegal salt length: 4 != 16 #85

Closed riegelTech closed 5 years ago

riegelTech commented 5 years ago

I use bcrypt.hashSync with a salt generated by openssl v1.0.2j, that contains "+" character : bcrypt.hashSync('some pass', "$2a$10$P3mJIQ+5mwrf3acCkCfoYg3WniW9TeD8mvfxufmhh3U=");

This throws an error :

Error: Illegal salt length: 4 != 16
    at _crypt (/home/baptiste/Projects/smc/node_modules/bcryptjs/dist/bcrypt.js:1183:18)
    at _hash (/home/baptiste/Projects/smc/node_modules/bcryptjs/dist/bcrypt.js:1345:27)
    at Object.bcrypt.hashSync (/home/baptiste/Projects/smc/node_modules/bcryptjs/dist/bcrypt.js:190:16)
    at repl:1:9
    at ContextifyScript.Script.runInThisContext (vm.js:50:33)
    at REPLServer.defaultEval (repl.js:240:29)
    at bound (domain.js:301:14)
    at REPLServer.runBound [as eval] (domain.js:314:12)
    at REPLServer.onLine (repl.js:441:10)
    at emitOne (events.js:121:20)

If I remove the "+" character it works fine, if I use bcrypt instead of bcryptjs, it works too.

riegelTech commented 5 years ago

It seems that bcrypt C++ implem uses same base64 dictionary, but ignores characters behind "+", that produces less secured hash without any error message.

So I am not convinced that it is a bug, especially if the fix consist in a change of the base64 dictionary and make new version of bcryptjs not compatible with old version.

Ruffio commented 5 years ago

Interesting. So to be 100% C++ compliant we have to lessen the security. So what is most important? To be compliant with the C++ implementation or the level of security?

riegelTech commented 5 years ago

There is no good solution. The best to do is probably to document this issue for hashSync function...