Merge branch 'master' into custom-forms-mjml

# Conflicts:
#	lib/tools.js
#	package.json

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

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]

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);

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);

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);

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 {

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 createDOMPurify = require('dompurify');
-let path = require('path');
 
 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(false, false, {
-    jsdom.env(html, (err, win) => {
+        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) {
 

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",
+    "redis": "^2.7.1",
-    "request": "^2.80.0",
+    "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"
   }

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');
                                 });

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