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/config/default.toml b/config/default.toml index bea73175..93fbab01 100644 --- a/config/default.toml +++ b/config/default.toml @@ -112,6 +112,8 @@ host="localhost" port=3002 baseDN="ou=users,dc=company" filter="(|(username={{username}})(mail={{username}}))" +#Username field in LDAP (uid/cn/username) +uidTag="username" passwordresetlink="" [postfixbounce] diff --git a/lib/models/campaigns.js b/lib/models/campaigns.js index b71d1bbf..98a2a896 100644 --- a/lib/models/campaigns.js +++ b/lib/models/campaigns.js @@ -619,6 +619,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); @@ -791,6 +794,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 f17c4886..02e7a4cd 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 15f069db..60bbbe3d 100644 --- a/lib/passport.js +++ b/lib/passport.js @@ -16,7 +16,9 @@ let LdapStrategy; try { LdapStrategy = require('passport-ldapjs').Strategy; // eslint-disable-line global-require } catch (E) { - // ignore + if (config.ldap.enabled) { + log.info('LDAP', 'Module "passport-ldapjs" not installed. LDAP auth will fail.'); + } } module.exports.csrfProtection = csrf({ @@ -80,27 +82,28 @@ if (config.ldap.enabled && LdapStrategy) { base: config.ldap.baseDN, search: { filter: config.ldap.filter, - attributes: ['username', 'mail'], + attributes: [config.ldap.uidTag, 'mail'], scope: 'sub' - } + }, + uidTag: config.ldap.uidTag }; passport.use(new LdapStrategy(opts, (profile, done) => { - users.findByUsername(profile.username, (err, user) => { + users.findByUsername(profile[config.ldap.uidTag], (err, user) => { if (err) { return done(err); } if (!user) { // password is empty for ldap - users.add(profile.username, '', profile.mail, (err, id) => { + users.add(profile[config.ldap.uidTag], '', profile.mail, (err, id) => { if (err) { return done(err); } return done(null, { id, - username: profile.username + username: profile[config.ldap.uidTag] }); }); } else { diff --git a/lib/tools.js b/lib/tools.js index 9e8591cc..e8e35f64 100644 --- a/lib/tools.js +++ b/lib/tools.js @@ -1,15 +1,17 @@ 'use strict'; +let fs = require('fs'); +let path = require('path'); let db = require('./db'); let slugify = require('slugify'); 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 fs = require('fs'); -let path = require('path'); +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']; @@ -24,6 +26,7 @@ module.exports = { formatMessage, getMessageLinks, prepareHtml, + purifyHTML, mergeTemplateIntoLayout, workers: new Set() }; @@ -172,7 +175,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); @@ -183,7 +186,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; }; @@ -199,8 +204,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); } @@ -228,6 +238,17 @@ function prepareHtml(html, callback) { }); } +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); +} + // TODO Simplify! function mergeTemplateIntoLayout(template, layout, callback) { @@ -274,4 +295,4 @@ function mergeTemplateIntoLayout(template, layout, callback) { } else { return done(template, layout); } -} +} \ No newline at end of file diff --git a/package.json b/package.json index 56395776..f5c7c6d5 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", @@ -47,6 +47,7 @@ "csurf": "^1.9.0", "csv-generate": "^1.0.0", "csv-parse": "^1.2.0", + "dompurify": "^0.8.5", "escape-html": "^1.0.3", "express": "^4.15.2", "express-session": "^1.15.1", @@ -64,7 +65,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,23 +77,23 @@ "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", "object-hash": "^1.1.7", - "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