diff --git a/CHANGELOG.txt b/CHANGELOG.md similarity index 85% rename from CHANGELOG.txt rename to CHANGELOG.md index 81317b68..c682ffb5 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## 1.23.0 2017-03-19 + + * Fixed security issue where description tags were able to include script tags. Reported by Andreas Lindh. Fixed with [ae6affda](https://github.com/andris9/mailtrain/commit/ae6affda8193f034e06f7e095ee23821a83d5190) + * Fixed security issue where templates that looked like file paths loaded content from arbitrary files. Reported by Andreas Lindh. Fixed with [0879fa41](https://github.com/andris9/mailtrain/commit/0879fa412a2d4a417aeca5cd5092a8f86531e7ef) + * Fixed security issue where users were able to use html tags in subscription values. Reported by Andreas Lindh. Fixed with [9d5fb816](https://github.com/andris9/mailtrain/commit/9d5fb816c937114966d4f589e1ad4e164ff3a187) + * Support for multiple HTML editors (Mosaico, GrapeJS, Summernote, HTML code) + ## 1.22.0 2017-03-02 * Reverted license back to GPL-v3 to support Mosaico diff --git a/lib/models/campaigns.js b/lib/models/campaigns.js index 2afe367c..62208707 100644 --- a/lib/models/campaigns.js +++ b/lib/models/campaigns.js @@ -655,6 +655,9 @@ module.exports.create = (campaign, opts, callback) => { Object.keys(campaign).forEach(key => { let value = typeof campaign[key] === 'number' ? campaign[key] : (campaign[key] || '').toString().trim(); key = tools.toDbKey(key); + if (key === 'description') { + value = tools.purifyHTML(value); + } if (allowedKeys.indexOf(key) >= 0 && keys.indexOf(key) < 0) { keys.push(key); values.push(value); @@ -827,6 +830,9 @@ module.exports.update = (id, updates, callback) => { Object.keys(campaign).forEach(key => { let value = typeof campaign[key] === 'number' ? campaign[key] : (campaign[key] || '').toString().trim(); key = tools.toDbKey(key); + if (key === 'description') { + value = tools.purifyHTML(value); + } if (allowedKeys.indexOf(key) >= 0 && keys.indexOf(key) < 0) { keys.push(key); values.push(value); diff --git a/lib/models/lists.js b/lib/models/lists.js index 4d58952c..97db2210 100644 --- a/lib/models/lists.js +++ b/lib/models/lists.js @@ -123,6 +123,9 @@ module.exports.create = (list, callback) => { Object.keys(list).forEach(key => { let value = list[key].trim(); key = tools.toDbKey(key); + if (key === 'description') { + value = tools.purifyHTML(value); + } if (allowedKeys.indexOf(key) >= 0) { keys.push(key); values.push(value); @@ -182,6 +185,9 @@ module.exports.update = (id, updates, callback) => { Object.keys(updates).forEach(key => { let value = updates[key].trim(); key = tools.toDbKey(key); + if (key === 'description') { + value = tools.purifyHTML(value); + } if (allowedKeys.indexOf(key) >= 0) { keys.push(key); values.push(value); diff --git a/lib/models/templates.js b/lib/models/templates.js index 0753a87f..4171ebb1 100644 --- a/lib/models/templates.js +++ b/lib/models/templates.js @@ -88,6 +88,9 @@ module.exports.create = (template, callback) => { Object.keys(template).forEach(key => { let value = template[key].trim(); key = tools.toDbKey(key); + if (key === 'description') { + value = tools.purifyHTML(value); + } if (allowedKeys.indexOf(key) >= 0) { keys.push(key); values.push(value); @@ -133,6 +136,9 @@ module.exports.update = (id, updates, callback) => { Object.keys(updates).forEach(key => { let value = updates[key].trim(); key = tools.toDbKey(key); + if (key === 'description') { + value = tools.purifyHTML(value); + } if (allowedKeys.indexOf(key) >= 0) { keys.push(key); values.push(value); diff --git a/lib/passport.js b/lib/passport.js index 60bbbe3d..04ef20ac 100644 --- a/lib/passport.js +++ b/lib/passport.js @@ -17,7 +17,7 @@ try { LdapStrategy = require('passport-ldapjs').Strategy; // eslint-disable-line global-require } catch (E) { if (config.ldap.enabled) { - log.info('LDAP', 'Module "passport-ldapjs" not installed. LDAP auth will fail.'); + log.info('LDAP', 'Module "passport-ldapjs" not installed. LDAP auth will fail.'); } } diff --git a/lib/tools.js b/lib/tools.js index 4fc8b396..e80b8d4a 100644 --- a/lib/tools.js +++ b/lib/tools.js @@ -6,8 +6,10 @@ let Isemail = require('isemail'); let urllib = require('url'); let juice = require('juice'); let jsdom = require('jsdom'); +let he = require('he'); let _ = require('./translate')._; let util = require('util'); +let createDOMPurify = require('dompurify'); let blockedUsers = ['abuse', 'admin', 'billing', 'compliance', 'devnull', 'dns', 'ftp', 'hostmaster', 'inoc', 'ispfeedback', 'ispsupport', 'listrequest', 'list', 'maildaemon', 'noc', 'noreply', 'noreply', 'null', 'phish', 'phishing', 'postmaster', 'privacy', 'registrar', 'root', 'security', 'spam', 'support', 'sysadmin', 'tech', 'undisclosedrecipients', 'unsubscribe', 'usenet', 'uucp', 'webmaster', 'www']; @@ -22,6 +24,7 @@ module.exports = { formatMessage, getMessageLinks, prepareHtml, + purifyHTML, workers: new Set() }; @@ -169,7 +172,7 @@ function getMessageLinks(serviceUrl, campaign, list, subscription) { }; } -function formatMessage(serviceUrl, campaign, list, subscription, message, filter) { +function formatMessage(serviceUrl, campaign, list, subscription, message, filter, isHTML) { filter = typeof filter === 'function' ? filter : (str => str); let links = getMessageLinks(serviceUrl, campaign, list, subscription); @@ -180,7 +183,9 @@ function formatMessage(serviceUrl, campaign, list, subscription, message, filter return links[key]; } if (subscription.mergeTags.hasOwnProperty(key)) { - return subscription.mergeTags[key]; + return isHTML ? he.encode(subscription.mergeTags[key], { + useNamedReferences: true + }) : subscription.mergeTags[key]; } return false; }; @@ -196,8 +201,13 @@ function prepareHtml(html, callback) { if (!(html || '').toString().trim()) { return callback(null, false); } - - jsdom.env(html, (err, win) => { + jsdom.env(false, false, { + html, + features: { + FetchExternalResources: false, // disables resource loading over HTTP / filesystem + ProcessExternalResources: false // do not execute JS within script blocks + } + }, (err, win) => { if (err) { return callback(err); } @@ -224,3 +234,14 @@ function prepareHtml(html, callback) { return callback(null, juice(preparedHtml)); }); } + +function purifyHTML(html) { + let win = jsdom.jsdom('', { + features: { + FetchExternalResources: false, // disables resource loading over HTTP / filesystem + ProcessExternalResources: false // do not execute JS within script blocks + } + }).defaultView; + let DOMPurify = createDOMPurify(win); + return DOMPurify.sanitize(html); +} diff --git a/package.json b/package.json index 04a898a6..fc40748e 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "mailtrain", "private": true, - "version": "1.22.0", + "version": "1.23.0", "description": "Self hosted email newsletter app", "main": "index.js", "scripts": { @@ -35,7 +35,7 @@ }, "dependencies": { "async": "^2.1.5", - "aws-sdk": "^2.24.0", + "aws-sdk": "^2.28.0", "bcrypt-nodejs": "0.0.3", "body-parser": "^1.17.1", "bounce-handler": "^7.3.2-fork.2", @@ -48,6 +48,7 @@ "csv-generate": "^1.0.0", "csv-parse": "^1.2.0", "device": "^0.3.8", + "dompurify": "^0.8.5", "escape-html": "^1.0.3", "express": "^4.15.2", "express-session": "^1.15.1", @@ -65,7 +66,7 @@ "is-url": "^1.2.2", "isemail": "^2.2.1", "jquery-file-upload-middleware": "^0.1.8", - "jsdom": "^9.11.0", + "jsdom": "^9.12.0", "juice": "^4.0.2", "libmime": "^3.1.0", "marked": "^0.3.6", @@ -76,22 +77,22 @@ "multer": "^1.3.0", "multiparty": "^4.1.3", "mysql": "^2.13.0", - "node-gettext": "^2.0.0-rc.0", + "node-gettext": "^2.0.0-rc.1", "node-mocks-http": "^1.6.1", - "nodemailer": "^3.1.5", + "nodemailer": "^3.1.7", "nodemailer-openpgp": "^1.0.2", "npmlog": "^4.0.2", - "openpgp": "^2.4.0", + "openpgp": "^2.5.1", "passport": "^0.3.2", "passport-local": "^1.0.0", "premailer-api": "^1.0.4", "redfour": "^1.0.0", - "redis": "^2.6.5", - "request": "^2.80.0", + "redis": "^2.7.1", + "request": "^2.81.0", "serve-favicon": "^2.4.1", "shortid": "^2.2.8", "slugify": "^1.1.0", - "smtp-server": "^2.0.2", + "smtp-server": "^2.0.3", "striptags": "^3.0.1", "toml": "^2.3.2" } diff --git a/routes/archive.js b/routes/archive.js index df384379..bd005db1 100644 --- a/routes/archive.js +++ b/routes/archive.js @@ -67,7 +67,7 @@ router.get('/:campaign/:list/:subscription', passport.csrfProtection, (req, res, let render = (view, layout) => { res.render(view, { layout, - message: renderTags ? tools.formatMessage(serviceUrl, campaign, list, subscription, html) : html, + message: renderTags ? tools.formatMessage(serviceUrl, campaign, list, subscription, html, false, true) : html, campaign, list, subscription, @@ -80,6 +80,9 @@ router.get('/:campaign/:list/:subscription', passport.csrfProtection, (req, res, res.render('partials/tracking-scripts', { layout: 'archive/layout-raw' }, (err, scripts) => { + if (err) { + return next(err); + } html = scripts ? html.replace(/<\/body\b/i, match => scripts + match) : html; render('archive/view-raw', 'archive/layout-raw'); }); diff --git a/services/sender.js b/services/sender.js index a76dafa4..400168bb 100644 --- a/services/sender.js +++ b/services/sender.js @@ -371,7 +371,7 @@ function formatMessage(message, callback) { let campaignAddress = [campaign.cid, list.cid, message.subscription.cid].join('.'); - let renderedHtml = renderTags ? tools.formatMessage(configItems.serviceUrl, campaign, list, message.subscription, html) : html; + let renderedHtml = renderTags ? tools.formatMessage(configItems.serviceUrl, campaign, list, message.subscription, html, false, true) : html; let renderedText = (text || '').trim() ? (renderTags ? tools.formatMessage(configItems.serviceUrl, campaign, list, message.subscription, text) : text) : htmlToText.fromString(renderedHtml, { wordwrap: 130