From 2c2a28c5b6d4bd2f1585543a96951b6360a55438 Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Fri, 8 Sep 2017 11:37:51 -0400 Subject: [PATCH 1/8] closes #5919 --- src/meta/blacklist.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/meta/blacklist.js b/src/meta/blacklist.js index 4a1eeeb71f..6d408d538b 100644 --- a/src/meta/blacklist.js +++ b/src/meta/blacklist.js @@ -20,7 +20,7 @@ Blacklist.load = function (callback) { Blacklist.get, Blacklist.validate, function (rules, next) { - winston.verbose('[meta/blacklist] Loading ' + rules.valid.length + ' blacklist rules'); + winston.verbose('[meta/blacklist] Loading ' + rules.valid.length + ' blacklist rule(s)' + (rules.duplicateCount > 0 ? ', ignored ' + rules.duplicateCount + ' duplicate(s)' : '')); if (rules.invalid.length) { winston.warn('[meta/blacklist] ' + rules.invalid.length + ' invalid blacklist rule(s) were ignored.'); } @@ -44,8 +44,8 @@ Blacklist.save = function (rules, callback) { db.set('ip-blacklist-rules', rules, next); }, function (next) { - Blacklist.load(next); pubsub.publish('blacklist:reload'); + setImmediate(next); }, ], callback); }; @@ -101,6 +101,7 @@ Blacklist.validate = function (rules, callback) { var ipv6 = []; var cidr = []; var invalid = []; + var duplicateCount = 0; var inlineCommentMatch = /#.*$/; var whitelist = ['127.0.0.1', '::1', '::ffff:0:127.0.0.1']; @@ -112,6 +113,16 @@ Blacklist.validate = function (rules, callback) { return rule.length && !rule.startsWith('#') ? rule : null; }).filter(Boolean); + // Filter out duplicates + rules = rules.filter(function (rule, index) { + const pass = rules.indexOf(rule) === index; + if (!pass) { + duplicateCount += 1; + } + + return pass; + }); + // Filter out invalid rules rules = rules.filter(function (rule) { var addr; @@ -157,6 +168,7 @@ Blacklist.validate = function (rules, callback) { cidr: cidr, valid: rules, invalid: invalid, + duplicateCount: duplicateCount, }); }; From a6b993ef6cd73b2cb903caeb15190f2ae47f6c70 Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Fri, 8 Sep 2017 11:55:37 -0400 Subject: [PATCH 2/8] updating 500-embed to load after page is fully loaded, #5733 --- src/views/500-embed.tpl | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/views/500-embed.tpl b/src/views/500-embed.tpl index b1045d431f..474e99a973 100644 --- a/src/views/500-embed.tpl +++ b/src/views/500-embed.tpl @@ -1,12 +1,14 @@ \ No newline at end of file From b56d6f8b5008f3b516c680525d3261aecab7d794 Mon Sep 17 00:00:00 2001 From: Baris Usakli Date: Fri, 8 Sep 2017 14:39:50 -0400 Subject: [PATCH 3/8] make call to db in /ping and /sping --- src/webserver.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/webserver.js b/src/webserver.js index 26f4820c15..7b5198378f 100644 --- a/src/webserver.js +++ b/src/webserver.js @@ -180,8 +180,15 @@ function setupExpressApp(app, callback) { setupAutoLocale(app, callback); } -function ping(req, res) { - res.status(200).send(req.path === '/sping' ? 'healthy' : '200'); +function ping(req, res, next) { + async.waterfall([ + function (next) { + db.getObject('config', next); + }, + function () { + res.status(200).send(req.path === '/sping' ? 'healthy' : '200'); + }, + ], next); } function setupFavicon(app) { From 9f4e92fa13a55fd89686282fa5da387a5c8578ce Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Tue, 12 Sep 2017 13:46:51 -0400 Subject: [PATCH 4/8] closes #5925 --- src/controllers/errors.js | 65 ++++++++++++++++++++++++++------------- 1 file changed, 43 insertions(+), 22 deletions(-) diff --git a/src/controllers/errors.js b/src/controllers/errors.js index 177bcaa769..a0d8faad21 100644 --- a/src/controllers/errors.js +++ b/src/controllers/errors.js @@ -3,6 +3,7 @@ var nconf = require('nconf'); var winston = require('winston'); var validator = require('validator'); +var plugins = require('../plugins'); exports.handleURIErrors = function (err, req, res, next) { // Handle cases where malformed URIs are passed in @@ -35,30 +36,50 @@ exports.handleURIErrors = function (err, req, res, next) { // this needs to have four arguments or express treats it as `(req, res, next)` // don't remove `next`! exports.handleErrors = function (err, req, res, next) { // eslint-disable-line no-unused-vars - switch (err.code) { - case 'EBADCSRFTOKEN': - winston.error(req.path + '\n', err.message); - return res.sendStatus(403); - case 'blacklisted-ip': - return res.status(403).type('text/plain').send(err.message); - } + var cases = { + EBADCSRFTOKEN: function () { + winston.error(req.path + '\n', err.message); + res.sendStatus(403); + }, + 'blacklisted-ip': function () { + res.status(403).type('text/plain').send(err.message); + }, + }; + var defaultHandler = function () { + // Display NodeBB error page + var status = parseInt(err.status, 10); + if ((status === 302 || status === 308) && err.path) { + return res.locals.isAPI ? res.set('X-Redirect', err.path).status(200).json(err.path) : res.redirect(err.path); + } - var status = parseInt(err.status, 10); - if ((status === 302 || status === 308) && err.path) { - return res.locals.isAPI ? res.set('X-Redirect', err.path).status(200).json(err.path) : res.redirect(err.path); - } + winston.error(req.path + '\n', err.stack); - winston.error(req.path + '\n', err.stack); + res.status(status || 500); - res.status(status || 500); + var path = String(req.path || ''); + if (res.locals.isAPI) { + res.json({ path: validator.escape(path), error: err.message }); + } else { + var middleware = require('../middleware'); + middleware.buildHeader(req, res, function () { + res.render('500', { path: validator.escape(path), error: validator.escape(String(err.message)) }); + }); + } + }; - var path = String(req.path || ''); - if (res.locals.isAPI) { - res.json({ path: validator.escape(path), error: err.message }); - } else { - var middleware = require('../middleware'); - middleware.buildHeader(req, res, function () { - res.render('500', { path: validator.escape(path), error: validator.escape(String(err.message)) }); - }); - } + plugins.fireHook('filter:error.handle', { + cases: cases, + }, function (_err, data) { + if (_err) { + // Assume defaults + winston.warn('[errors/handle] Unable to retrieve plugin handlers for errors: ' + _err.message); + data.cases = cases; + } + + if (data.cases.hasOwnProperty(err.code)) { + data.cases[err.code](err, res, res, defaultHandler); + } else { + defaultHandler(); + } + }); }; From 8b0e6611d954ffac554577b0ebe2021a8badcaba Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Tue, 12 Sep 2017 18:55:47 -0400 Subject: [PATCH 5/8] fixed incorrect parameter passed into hook --- src/controllers/errors.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/controllers/errors.js b/src/controllers/errors.js index a0d8faad21..96bfda203c 100644 --- a/src/controllers/errors.js +++ b/src/controllers/errors.js @@ -77,7 +77,7 @@ exports.handleErrors = function (err, req, res, next) { // eslint-disable-line n } if (data.cases.hasOwnProperty(err.code)) { - data.cases[err.code](err, res, res, defaultHandler); + data.cases[err.code](err, req, res, defaultHandler); } else { defaultHandler(); } From 211482bbc0f5218d0d96657b9da3a1a42e8676b1 Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Wed, 13 Sep 2017 12:09:31 -0400 Subject: [PATCH 6/8] fixes #5927 --- public/src/app.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/public/src/app.js b/public/src/app.js index 913360c615..d6acad4554 100644 --- a/public/src/app.js +++ b/public/src/app.js @@ -439,6 +439,9 @@ app.cacheBuster = null; .replace('{pageTitle}', function () { return title; }) .replace('{browserTitle}', function () { return config.browserTitle; }); + // Allow translation strings in title on ajaxify (#5927) + title = title.replace(/[/g, '[').replace(/]/g, ']'); + translator.translate(title, function (translated) { titleObj.titles[0] = translated; app.alternatingTitle(''); From 514317ab20c625bb703dc26fab05e1d372e6697f Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Wed, 13 Sep 2017 17:06:00 -0400 Subject: [PATCH 7/8] using translator.unescape instead of replaces --- public/src/app.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/src/app.js b/public/src/app.js index d6acad4554..421cf71943 100644 --- a/public/src/app.js +++ b/public/src/app.js @@ -440,7 +440,7 @@ app.cacheBuster = null; .replace('{browserTitle}', function () { return config.browserTitle; }); // Allow translation strings in title on ajaxify (#5927) - title = title.replace(/[/g, '[').replace(/]/g, ']'); + title = translator.unescape(title); translator.translate(title, function (translated) { titleObj.titles[0] = translated; From dea372a5c56f8dadfd79b4ef7c28768feb1a5586 Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Thu, 14 Sep 2017 10:15:05 -0400 Subject: [PATCH 8/8] fixes #5932 --- src/middleware/index.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/middleware/index.js b/src/middleware/index.js index 511ae1ddd4..d7f1ce1a73 100644 --- a/src/middleware/index.js +++ b/src/middleware/index.js @@ -9,6 +9,7 @@ var nconf = require('nconf'); var ensureLoggedIn = require('connect-ensure-login'); var toobusy = require('toobusy-js'); var Benchpress = require('benchpressjs'); +var LRU = require('lru-cache'); var plugins = require('../plugins'); var meta = require('../meta'); @@ -23,6 +24,10 @@ var controllers = { helpers: require('../controllers/helpers'), }; +var delayCache = LRU({ + maxAge: 1000 * 60, +}); + var middleware = module.exports; middleware.applyCSRF = csrf(); @@ -186,6 +191,14 @@ middleware.processTimeagoLocales = function (req, res, next) { middleware.delayLoading = function (req, res, next) { // Introduces an artificial delay during load so that brute force attacks are effectively mitigated + + // Add IP to cache so if too many requests are made, subsequent requests are blocked for a minute + var timesSeen = delayCache.get(req.ip) || 0; + if (timesSeen > 10) { + return res.sendStatus(429); + } + delayCache.set(req.ip, timesSeen += 1); + setTimeout(next, 1000); };