From ecfca21abe9b3330a75bf86f65668a74b46e6a53 Mon Sep 17 00:00:00 2001 From: Peter Jaszkowiak Date: Sat, 15 Apr 2017 00:50:12 -0600 Subject: [PATCH 1/3] Up composer --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index e846dfb8ce..2db62f41fd 100644 --- a/package.json +++ b/package.json @@ -54,7 +54,7 @@ "morgan": "^1.3.2", "mousetrap": "^1.5.3", "nconf": "~0.8.2", - "nodebb-plugin-composer-default": "4.4.4", + "nodebb-plugin-composer-default": "4.4.6", "nodebb-plugin-dbsearch": "1.0.5", "nodebb-plugin-emoji-extended": "1.1.1", "nodebb-plugin-emoji-one": "1.1.5", From 0fe10f5e866843485a71f05f32b99b72bd575c6a Mon Sep 17 00:00:00 2001 From: Peter Jaszkowiak Date: Sat, 15 Apr 2017 01:38:42 -0600 Subject: [PATCH 2/3] Escape topic titles at the source, deduplicate --- public/src/modules/translator.js | 6 ++++- src/categories/recentreplies.js | 4 +-- src/controllers/category.js | 3 --- src/controllers/topics.js | 4 +-- src/posts/summary.js | 13 +++++---- src/socket.io/admin/rooms.js | 2 +- src/socket.io/posts/tools.js | 2 +- src/socket.io/topics/tools.js | 2 +- src/topics/create.js | 2 +- src/topics/data.js | 46 +++++++++++++++++++++++++++++--- 10 files changed, 59 insertions(+), 25 deletions(-) diff --git a/public/src/modules/translator.js b/public/src/modules/translator.js index b0d1ad5cb8..2e9dcafb9c 100644 --- a/public/src/modules/translator.js +++ b/public/src/modules/translator.js @@ -50,6 +50,7 @@ /** * Construct a new Translator object * @param {string} language - Language code for this translator instance + * @exports translator.Translator */ function Translator(language) { var self = this; @@ -283,7 +284,7 @@ } var argsToTranslate = args.map(function (arg) { - return string(arg).collapseWhitespace().decodeHTMLEntities().escapeHTML().s; + return string(arg).collapseWhitespace().decodeHTMLEntities().escapeHTML().s.replace(/&/g, '&'); }).map(function (arg) { return self.translate(arg); }); @@ -443,6 +444,9 @@ return Translator; }()); + /** + * @exports translator + */ var adaptor = { /** * The Translator class diff --git a/src/categories/recentreplies.js b/src/categories/recentreplies.js index 1908db45e6..595cba3f9e 100644 --- a/src/categories/recentreplies.js +++ b/src/categories/recentreplies.js @@ -3,7 +3,6 @@ var async = require('async'); var winston = require('winston'); -var validator = require('validator'); var _ = require('underscore'); var db = require('../database'); @@ -11,7 +10,6 @@ var posts = require('../posts'); var topics = require('../topics'); var privileges = require('../privileges'); var batch = require('../batch'); -var translator = require('../translator'); module.exports = function (Categories) { Categories.getRecentReplies = function (cid, uid, count, callback) { @@ -136,7 +134,7 @@ module.exports = function (Categories) { teaser.user.uid = undefined; teaser.topic = { slug: topicData[index].slug, - title: translator.escape(validator.escape(String(topicData[index].title))), + title: topicData[index].title, }; } }); diff --git a/src/controllers/category.js b/src/controllers/category.js index 602f158eeb..114fd8ba63 100644 --- a/src/controllers/category.js +++ b/src/controllers/category.js @@ -161,9 +161,6 @@ categoryController.get = function (req, res, callback) { return callback(err); } - categoryData.topics.forEach(function (topic) { - topic.title = translator.escape(topic.title); - }); categoryData.description = translator.escape(categoryData.description); categoryData.privileges = userPrivileges; categoryData.showSelect = categoryData.privileges.editable; diff --git a/src/controllers/topics.js b/src/controllers/topics.js index 8f767ad36c..4fe1bf96d5 100644 --- a/src/controllers/topics.js +++ b/src/controllers/topics.js @@ -14,7 +14,6 @@ var plugins = require('../plugins'); var helpers = require('./helpers'); var pagination = require('../pagination'); var utils = require('../utils'); -var translator = require('../translator'); var topicsController = {}; @@ -130,14 +129,13 @@ topicsController.get = function (req, res, callback) { plugins.fireHook('filter:controllers.topic.get', { topicData: topicData, uid: req.uid }, next); }, function (data, next) { - data.topicData.title = translator.escape(data.topicData.title); var breadcrumbs = [ { text: data.topicData.category.name, url: nconf.get('relative_path') + '/category/' + data.topicData.category.slug, }, { - text: translator.escape(data.topicData.title), + text: data.topicData.title, }, ]; diff --git a/src/posts/summary.js b/src/posts/summary.js index 0eaadaca94..065007dca8 100644 --- a/src/posts/summary.js +++ b/src/posts/summary.js @@ -5,12 +5,11 @@ var async = require('async'); var validator = require('validator'); var S = require('string'); -var db = require('../database'); +var topics = require('../topics'); var user = require('../user'); var plugins = require('../plugins'); var categories = require('../categories'); var utils = require('../utils'); -var translator = require('../translator'); module.exports = function (Posts) { Posts.getPostSummaryByPids = function (pids, uid, options, callback) { @@ -39,8 +38,8 @@ module.exports = function (Posts) { if (uids.indexOf(posts[i].uid) === -1) { uids.push(posts[i].uid); } - if (topicKeys.indexOf('topic:' + posts[i].tid) === -1) { - topicKeys.push('topic:' + posts[i].tid); + if (topicKeys.indexOf(posts[i].tid) === -1) { + topicKeys.push(posts[i].tid); } }); async.parallel({ @@ -111,15 +110,15 @@ module.exports = function (Posts) { }, callback); } - function getTopicAndCategories(topicKeys, callback) { - db.getObjectsFields(topicKeys, ['uid', 'tid', 'title', 'cid', 'slug', 'deleted', 'postcount', 'mainPid'], function (err, topics) { + function getTopicAndCategories(tids, callback) { + topics.getTopicsFields(tids, ['uid', 'tid', 'title', 'cid', 'slug', 'deleted', 'postcount', 'mainPid'], function (err, topics) { if (err) { return callback(err); } var cids = topics.map(function (topic) { if (topic) { - topic.title = translator.escape(validator.escape(String(topic.title))); + topic.title = String(topic.title); topic.deleted = parseInt(topic.deleted, 10) === 1; } return topic && topic.cid; diff --git a/src/socket.io/admin/rooms.js b/src/socket.io/admin/rooms.js index 544bdc04da..f7a16bf85d 100644 --- a/src/socket.io/admin/rooms.js +++ b/src/socket.io/admin/rooms.js @@ -109,7 +109,7 @@ SocketRooms.getAll = function (socket, data, callback) { topTenTopics.forEach(function (topic, index) { totals.topics[topic.tid] = { value: topic.count || 0, - title: validator.escape(String(titles[index].title)), + title: String(titles[index].title), }; }); next(null, totals); diff --git a/src/socket.io/posts/tools.js b/src/socket.io/posts/tools.js index ee393a42b9..8d7a91ae9a 100644 --- a/src/socket.io/posts/tools.js +++ b/src/socket.io/posts/tools.js @@ -169,7 +169,7 @@ module.exports = function (SocketPosts) { uid: socket.uid, pid: data.pid, ip: socket.ip, - title: validator.escape(String(title)), + title: String(title), }, next); }, ], callback); diff --git a/src/socket.io/topics/tools.js b/src/socket.io/topics/tools.js index 74cdb68e7e..e08b31e6e4 100644 --- a/src/socket.io/topics/tools.js +++ b/src/socket.io/topics/tools.js @@ -114,7 +114,7 @@ module.exports = function (SocketTopics) { uid: socket.uid, ip: socket.ip, tid: tid, - title: validator.escape(String(title)), + title: String(title), }, next); }, ], callback); diff --git a/src/topics/create.js b/src/topics/create.js index 2c7f87741b..827465478b 100644 --- a/src/topics/create.js +++ b/src/topics/create.js @@ -323,7 +323,7 @@ module.exports = function (Topics) { postData.display_move_tools = true; postData.selfPost = false; postData.timestampISO = utils.toISOString(postData.timestamp); - postData.topic.title = validator.escape(String(postData.topic.title)); + postData.topic.title = String(postData.topic.title); next(null, postData); }, diff --git a/src/topics/data.js b/src/topics/data.js index 8df112dcbf..3a2a6e75d0 100644 --- a/src/topics/data.js +++ b/src/topics/data.js @@ -5,14 +5,43 @@ var validator = require('validator'); var db = require('../database'); var categories = require('../categories'); var utils = require('../utils'); +var translator = require('../translator'); + +function escapeTitle(topicData) { + if (!topicData) { + return; + } + if (topicData.title) { + topicData.title = translator.escape(validator.escape(topicData.title)); + } + if (topicData.titleRaw) { + topicData.titleRaw = translator.escape(topicData.titleRaw); + } +} module.exports = function (Topics) { Topics.getTopicField = function (tid, field, callback) { - db.getObjectField('topic:' + tid, field, callback); + db.getObjectField('topic:' + tid, field, function (err, value) { + if (err) { + return callback(err); + } + + if (field === 'title') { + value = translator.escape(validator.escape(value)); + } + callback(null, value); + }); }; Topics.getTopicFields = function (tid, fields, callback) { - db.getObjectFields('topic:' + tid, fields, callback); + db.getObjectFields('topic:' + tid, fields, function (err, topic) { + if (err) { + return callback(err); + } + + escapeTitle(topic); + callback(null, topic); + }); }; Topics.getTopicsFields = function (tids, fields, callback) { @@ -22,7 +51,14 @@ module.exports = function (Topics) { var keys = tids.map(function (tid) { return 'topic:' + tid; }); - db.getObjectsFields(keys, fields, callback); + db.getObjectsFields(keys, fields, function (err, topics) { + if (err) { + return callback(err); + } + + topics.forEach(escapeTitle); + callback(null, topics); + }); }; Topics.getTopicData = function (tid, callback) { @@ -57,8 +93,10 @@ module.exports = function (Topics) { if (!topic) { return; } + topic.titleRaw = topic.title; - topic.title = validator.escape(String(topic.title)); + topic.title = String(topic.title); + escapeTitle(topic); topic.timestampISO = utils.toISOString(topic.timestamp); topic.lastposttimeISO = utils.toISOString(topic.lastposttime); } From e382bca610191f4fc0880bf198e18357058b9548 Mon Sep 17 00:00:00 2001 From: Peter Jaszkowiak Date: Sat, 15 Apr 2017 04:22:28 -0600 Subject: [PATCH 3/3] Fix tests --- src/socket.io/admin/rooms.js | 2 +- src/socket.io/posts/tools.js | 1 - src/socket.io/topics/tools.js | 1 - src/topics/data.js | 4 ++-- 4 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/socket.io/admin/rooms.js b/src/socket.io/admin/rooms.js index f7a16bf85d..70b908d4dc 100644 --- a/src/socket.io/admin/rooms.js +++ b/src/socket.io/admin/rooms.js @@ -5,7 +5,7 @@ var async = require('async'); var os = require('os'); var nconf = require('nconf'); var winston = require('winston'); -var validator = require('validator'); + var topics = require('../../topics'); var pubsub = require('../../pubsub'); diff --git a/src/socket.io/posts/tools.js b/src/socket.io/posts/tools.js index 8d7a91ae9a..c1ad05b119 100644 --- a/src/socket.io/posts/tools.js +++ b/src/socket.io/posts/tools.js @@ -1,7 +1,6 @@ 'use strict'; var async = require('async'); -var validator = require('validator'); var posts = require('../../posts'); var topics = require('../../topics'); diff --git a/src/socket.io/topics/tools.js b/src/socket.io/topics/tools.js index e08b31e6e4..7302a5ad04 100644 --- a/src/socket.io/topics/tools.js +++ b/src/socket.io/topics/tools.js @@ -1,7 +1,6 @@ 'use strict'; var async = require('async'); -var validator = require('validator'); var topics = require('../../topics'); var events = require('../../events'); diff --git a/src/topics/data.js b/src/topics/data.js index 3a2a6e75d0..d0a6208632 100644 --- a/src/topics/data.js +++ b/src/topics/data.js @@ -12,7 +12,7 @@ function escapeTitle(topicData) { return; } if (topicData.title) { - topicData.title = translator.escape(validator.escape(topicData.title)); + topicData.title = translator.escape(validator.escape(topicData.title.toString())); } if (topicData.titleRaw) { topicData.titleRaw = translator.escape(topicData.titleRaw); @@ -27,7 +27,7 @@ module.exports = function (Topics) { } if (field === 'title') { - value = translator.escape(validator.escape(value)); + value = translator.escape(validator.escape(String(value))); } callback(null, value); });