From 08f1750d9450df834a0cf19d303e3c2f9e5da565 Mon Sep 17 00:00:00 2001 From: Rupert Muchembled Date: Wed, 5 Nov 2014 20:33:01 +0000 Subject: [PATCH] Correctly encode and decode password salt 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. --- modules/users/server/models/user.server.model.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/users/server/models/user.server.model.js b/modules/users/server/models/user.server.model.js index d92d29aff9..f4d06c6df2 100644 --- a/modules/users/server/models/user.server.model.js +++ b/modules/users/server/models/user.server.model.js @@ -100,7 +100,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); } @@ -112,7 +112,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; }