From 520a8bc7a72e0bb82777325936f3863cb2ed6a35 Mon Sep 17 00:00:00 2001 From: palmoni5 Date: Sat, 5 Sep 2026 23:01:30 +0300 Subject: [PATCH] fix(security): harden backup code login path - Store backup codes as SHA-256 hashes instead of plain text. Codes that were generated before this change are still accepted (matched as plain text) so existing users are not locked out. - Generate codes with crypto.randomBytes (12 hex chars). The previous implementation only stripped the first dash from the UUID, which left a 10-character code. - Rate-limit POST /login/2fa/backup the same way as the TOTP path: one attempt at a time per uid, 2s delay on failure, 10s penalty when spammed. - Apply CSRF protection to POST /login/2fa/totp and POST /login/2fa/backup. The templates were already rendering the token but under a field name (`csrf`) that core never reads; rename it to `csrf_token`. - Normalise submitted backup codes (trim, lowercase, strip spaces/dashes). --- lib/controllers.js | 11 ++++++++++ library.js | 34 +++++++++++++++++++++++-------- static/templates/login-backup.tpl | 2 +- static/templates/login-totp.tpl | 2 +- 4 files changed, 38 insertions(+), 11 deletions(-) diff --git a/lib/controllers.js b/lib/controllers.js index 0507855..7bd710d 100644 --- a/lib/controllers.js +++ b/lib/controllers.js @@ -199,15 +199,26 @@ Controllers.generateBackupCodes = async (req, res) => { }; Controllers.processBackup = async (req, res, next) => { + const count = await db.incrObjectField('locks', `backup:${req.uid}`); + if (count > 1) { + req.flash('error', '[[error:api.429]]'); + await wait(10000); // 10s for spamming + return res.redirect(`${nconf.get('relative_path')}/login/2fa/backup`); + } + try { const success = await parent.useBackupCode(req.body.code, req.user.uid); if (!success) { req.flash('error', '[[2factor:backup.failure]]'); + await wait(2000); + await db.deleteObjectField('locks', `backup:${req.uid}`); return res.redirect(`${nconf.get('relative_path')}/login/2fa/backup`); } // Success! + await db.deleteObjectField('locks', `backup:${req.uid}`); next(); } catch (err) { + await db.deleteObjectField('locks', `backup:${req.uid}`); req.flash('error', err.message); res.redirect(`${nconf.get('relative_path')}/login/2fa/backup`); } diff --git a/library.js b/library.js index 525a6fb..e106079 100644 --- a/library.js +++ b/library.js @@ -5,6 +5,7 @@ const passportTotp = require('passport-totp').Strategy; const notp = require('notp'); const { Fido2Lib } = require('fido2-lib'); const base64url = require('base64url'); +const crypto = require('crypto'); const db = nodebb.require('./src/database'); const nconf = nodebb.require('nconf'); @@ -14,7 +15,6 @@ const meta = nodebb.require('./src/meta'); const groups = nodebb.require('./src/groups'); const plugins = nodebb.require('./src/plugins'); const notifications = nodebb.require('./src/notifications'); -const utils = nodebb.require('./src/utils'); const routeHelpers = nodebb.require('./src/routes/helpers'); const controllerHelpers = nodebb.require('./src/controllers/helpers'); const SocketPlugins = nodebb.require('./src/socket.io/plugins'); @@ -57,7 +57,7 @@ plugin.init = async (params) => { // 2fa Login hostHelpers.setupPageRoute(router, '/login/2fa', [hostMiddleware.ensureLoggedIn], controllers.renderChoices); hostHelpers.setupPageRoute(router, '/login/2fa/totp', [hostMiddleware.ensureLoggedIn], controllers.renderTotpChallenge); - router.post('/login/2fa/totp', hostMiddleware.ensureLoggedIn, controllers.processTotpLogin, (req, res) => { + router.post('/login/2fa/totp', hostMiddleware.ensureLoggedIn, hostMiddleware.applyCSRF, controllers.processTotpLogin, (req, res) => { req.session.tfa = true; const now = Date.now(); req.session.meta.datetime = now; @@ -72,7 +72,7 @@ plugin.init = async (params) => { // 2fa backups codes hostHelpers.setupPageRoute(router, '/login/2fa/backup', [hostMiddleware.ensureLoggedIn], controllers.renderBackup); - router.post('/login/2fa/backup', hostMiddleware.ensureLoggedIn, controllers.processBackup, (req, res) => { + router.post('/login/2fa/backup', hostMiddleware.ensureLoggedIn, hostMiddleware.applyCSRF, controllers.processBackup, (req, res) => { req.session.tfa = true; res.redirect(guard(nconf.get('relative_path') + (req.query.next || '/'))); }); @@ -329,6 +329,10 @@ plugin.hasKey = async (uid) => { return hasTotp || hasAuthn; }; +function hashBackupCode(code) { + return crypto.createHash('sha256').update(code).digest('hex'); +} + plugin.hasBackupCodes = async uid => db.exists(`2factor:uid:${uid}:backupCodes`); plugin.countBackupCodes = async uid => db.setCount(`2factor:uid:${uid}:backupCodes`); @@ -336,15 +340,13 @@ plugin.countBackupCodes = async uid => db.setCount(`2factor:uid:${uid}:backupCod plugin.generateBackupCodes = async (uid) => { const set = `2factor:uid:${uid}:backupCodes`; const codes = []; - let code; for (let x = 0; x < 5; x++) { - code = utils.generateUUID().replace('-', '').slice(0, 10); - codes.push(code); + codes.push(crypto.randomBytes(6).toString('hex')); } await db.delete(set); // Invalidate all old codes - await db.setAdd(set, codes); // Save new codes + await db.setAdd(set, codes.map(hashBackupCode)); // Save hashes only const notification = await notifications.create({ bodyShort: '[[2factor:notification.backupCode.generated]]', @@ -364,10 +366,24 @@ plugin.generateBackupCodes = async (uid) => { plugin.useBackupCode = async (code, uid) => { const set = `2factor:uid:${uid}:backupCodes`; - const valid = await db.isSetMember(set, code); + if (typeof code !== 'string') { + return false; + } + code = code.trim().toLowerCase().replace(/[\s-]/g, ''); + if (!code) { + return false; + } + + const hashed = hashBackupCode(code); + // Codes generated before hashing was introduced are stored in plain text + const [validHashed, validLegacy] = await Promise.all([ + db.isSetMember(set, hashed), + db.isSetMember(set, code), + ]); + const valid = validHashed || validLegacy; if (valid) { // Invalidate this backup code - await db.setRemove(set, code); + await db.setRemove(set, validHashed ? hashed : code); const notification = await notifications.create({ bodyShort: '[[2factor:notification.backupCode.used]]', diff --git a/static/templates/login-backup.tpl b/static/templates/login-backup.tpl index 83b527c..f3275c8 100644 --- a/static/templates/login-backup.tpl +++ b/static/templates/login-backup.tpl @@ -17,7 +17,7 @@
- +
diff --git a/static/templates/login-totp.tpl b/static/templates/login-totp.tpl index b87a114..de91f99 100644 --- a/static/templates/login-totp.tpl +++ b/static/templates/login-totp.tpl @@ -20,7 +20,7 @@ - +