Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

Commit

Permalink
Correctly encode and decode password salt
Browse files Browse the repository at this point in the history
The user password salt should be encoded with Base64 before being saved
to the database.

The current code adds an unecessary step of converting the result of
crypto.randomBytes() (which already returns a SlowBuffer) to a Base64
string and back again to a Buffer, and misses the final step of
converting the Buffer's bytes back to a Base64 string.

Because of this, the salt stored in the database is garbled. This is
inconvenient when manipulating the data in a terminal or text editor.

When generating the password hash, the crypto.pbkdf2Sync() method
creates a new Buffer directly from the data supplied. Due to the
incorrect encoding of the salt, entropy is lost at this step,
weakening the security of stored passwords against brute force attacks.
  • Loading branch information
rmuch committed Nov 5, 2014
1 parent 42931d3 commit 325a19e
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions app/models/user.server.model.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ var UserSchema = new Schema({
*/
UserSchema.pre('save', function(next) {
if (this.password && this.password.length > 6) {
this.salt = new Buffer(crypto.randomBytes(16).toString('base64'), 'base64');
this.salt = crypto.randomBytes(16).toString('base64');
this.password = this.hashPassword(this.password);
}

Expand All @@ -108,7 +108,7 @@ UserSchema.pre('save', function(next) {
*/
UserSchema.methods.hashPassword = function(password) {
if (this.salt && password) {
return crypto.pbkdf2Sync(password, this.salt, 10000, 64).toString('base64');
return crypto.pbkdf2Sync(password, new Buffer(this.salt, 'base64'), 10000, 64).toString('base64');
} else {
return password;
}
Expand Down

0 comments on commit 325a19e

Please sign in to comment.