From 77ec41bb6f542077503106cacc1dbd28118c50b4 Mon Sep 17 00:00:00 2001 From: Felix Hammerl Date: Wed, 24 Feb 2016 10:13:25 +0100 Subject: Issue #617: Sanitize received content Sanitizes received HTML content with DOMPurify, making it safe for displaying and templating. Sanitizes received plain text content by encoding every single character as HTML entity. --- web-ui/app/js/helpers/sanitizer.js | 108 +++++++++++++++++++++++++++ web-ui/app/js/helpers/view_helper.js | 37 +-------- web-ui/app/js/mail_view/ui/mail_view.js | 1 + web-ui/app/js/main.js | 2 + web-ui/bower.json | 4 +- web-ui/karma.conf.js | 2 + web-ui/test/spec/helpers/sanitizer.spec.js | 49 ++++++++++++ web-ui/test/spec/helpers/view_helper.spec.js | 7 -- web-ui/test/test-main.js | 3 +- 9 files changed, 171 insertions(+), 42 deletions(-) create mode 100644 web-ui/app/js/helpers/sanitizer.js create mode 100644 web-ui/test/spec/helpers/sanitizer.spec.js (limited to 'web-ui') diff --git a/web-ui/app/js/helpers/sanitizer.js b/web-ui/app/js/helpers/sanitizer.js new file mode 100644 index 00000000..eea1f0f7 --- /dev/null +++ b/web-ui/app/js/helpers/sanitizer.js @@ -0,0 +1,108 @@ +/* + * Copyright (c) 2016 ThoughtWorks, Inc. + * + * Pixelated is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Pixelated is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with Pixelated. If not, see . + */ + +define(['DOMPurify', 'he'], function (DOMPurify, he) { + 'use strict'; + + /** + * Sanitizes a mail body to safe-to-display HTML + */ + var sanitizer = {}; + + /** + * Adds html line breaks to a plaintext with line breaks (incl carriage return) + * + * @param {string} textPlainBody Plaintext input + * @returns {string} Plaintext with HTML line breals (
) + */ + sanitizer.addLineBreaks = function (textPlainBody) { + return textPlainBody.replace(/(\r)?\n/g, '
').replace(/( )? /g, '
'); + }; + + /** + * Runs a given dirty body through DOMPurify, thereby removing + * potentially hazardous XSS attacks. Please be advised that this + * will not act as a privacy leak prevention. Contained contents + * will still point to remote sources. + * + * For future reference: Running DOMPurify with these parameters + * can help mitigate some of the most widely used privacy leaks. + * FORBID_TAGS: ['style', 'svg', 'audio', 'video', 'math'], + * FORBID_ATTR: ['src'] + * + * @param {string} dirtyBody The unsanitized string + * @return {string} Safe-to-display HTML string + */ + sanitizer.purifyHtml = function (dirtyBody) { + return DOMPurify.sanitize(dirtyBody, { + SAFE_FOR_JQUERY: true, + SAFE_FOR_TEMPLATES: true + }); + }; + + /** + * Runs a given dirty body through he, thereby encoding everything + * as HTML entities. + * + * @param {string} dirtyBody The unsanitized string + * @return {string} Safe-to-display HTML string + */ + sanitizer.purifyText = function (dirtyBody) { + return he.encode(dirtyBody, { + encodeEverything: true + }); + }; + + /** + * Calls #purify and #addLineBreaks to turn untrusted mail body content + * into safe-to-display HTML. + * + * NB: HTML content is preferred to plaintext content. + * + * @param {object} mail Pixelated Mail Object + * @return {string} Safe-to-display HTML string + */ + sanitizer.sanitize = function (mail) { + var body; + + if (mail.htmlBody) { + body = this.purifyHtml(mail.htmlBody); + } else { + body = this.purifyText(mail.textPlainBody); + body = this.addLineBreaks(body); + } + + return body; + }; + + /** + * Add hooks to DOMPurify for opening links in new windows + */ + DOMPurify.addHook('afterSanitizeAttributes', function (node) { + // set all elements owning target to target=_blank + if ('target' in node) { + node.setAttribute('target', '_blank'); + } + + // set non-HTML/MathML links to xlink:show=new + if (!node.hasAttribute('target') && (node.hasAttribute('xlink:href') || node.hasAttribute('href'))) { + node.setAttribute('xlink:show', 'new'); + } + }); + + return sanitizer; +}); diff --git a/web-ui/app/js/helpers/view_helper.js b/web-ui/app/js/helpers/view_helper.js index e4e9277d..e8d517a5 100644 --- a/web-ui/app/js/helpers/view_helper.js +++ b/web-ui/app/js/helpers/view_helper.js @@ -17,12 +17,12 @@ define( [ 'helpers/contenttype', - 'lib/html_whitelister', 'views/i18n', 'quoted-printable/quoted-printable', - 'utf8/utf8' + 'utf8/utf8', + 'helpers/sanitizer' ], - function(contentType, htmlWhitelister, i18n, quotedPrintable, utf8) { + function(contentType, i18n, quotedPrintable, utf8, sanitizer) { 'use strict'; function formatStatusClasses(ss) { @@ -31,37 +31,8 @@ define( }).join(' '); } - function addParagraphsToPlainText(textPlainBody) { - return textPlainBody.replace(/^(.*?)$/mg, '$1
'); - } - - function escapeHtmlTags(body) { - - var escapeIndex = { - '&': '&', - '<': '<', - '>': '>', - '"': '"', - '\'':''', - '/': '/' - }; - - return body.replace(/["'<>\/&]/g, function(char){ - return escapeIndex[char]; - }); - - } - - function escapeHtmlAndAddParagraphs(body) { - var escapedBody = escapeHtmlTags(body); - return addParagraphsToPlainText(escapedBody); - } - function formatMailBody(mail) { - var body = mail.htmlBody ? - htmlWhitelister.sanitize(mail.htmlBody, htmlWhitelister.tagPolicy) : - escapeHtmlAndAddParagraphs(mail.textPlainBody); - return $('
' + body + '
'); + return sanitizer.sanitize(mail); } function moveCaretToEnd(el) { diff --git a/web-ui/app/js/mail_view/ui/mail_view.js b/web-ui/app/js/mail_view/ui/mail_view.js index d4f5dd9e..8465b45a 100644 --- a/web-ui/app/js/mail_view/ui/mail_view.js +++ b/web-ui/app/js/mail_view/ui/mail_view.js @@ -72,6 +72,7 @@ define( })); this.$node.find('.bodyArea').html(viewHelpers.formatMailBody(data.mail)); + this.trigger(document, events.search.highlightResults, {where: '.bodyArea'}); this.trigger(document, events.search.highlightResults, {where: '.subjectArea'}); this.trigger(document, events.search.highlightResults, {where: '.msg-header .recipients'}); diff --git a/web-ui/app/js/main.js b/web-ui/app/js/main.js index 5fb2e46f..e093e790 100644 --- a/web-ui/app/js/main.js +++ b/web-ui/app/js/main.js @@ -22,6 +22,8 @@ requirejs.config({ 'page': 'js/page', 'feedback': 'js/feedback', 'flight': 'bower_components/flight', + 'DOMPurify': 'bower_components/DOMPurify/dist/purify.min', + 'he': 'bower_components/he/he', 'hbs': 'js/generated/hbs', 'helpers': 'js/helpers', 'lib': 'js/lib', diff --git a/web-ui/bower.json b/web-ui/bower.json index 261f6e90..263ac2e4 100644 --- a/web-ui/bower.json +++ b/web-ui/bower.json @@ -15,7 +15,9 @@ "utf8": "~2.1.1", "modernizr": "~2.8.3", "jquery-file-upload": "~9.11.2", - "jquery-ui": "~1.11.4" + "jquery-ui": "~1.11.4", + "DOMPurify": "~0.7.4", + "he": "~0.5.0" }, "devDependencies": { "handlebars": "2.0.0", diff --git a/web-ui/karma.conf.js b/web-ui/karma.conf.js index a59b1d4f..e31262ff 100644 --- a/web-ui/karma.conf.js +++ b/web-ui/karma.conf.js @@ -36,6 +36,8 @@ module.exports = function (config) { 'node_modules/karma-requirejs/lib/adapter.js', // loaded with require + {pattern: 'app/bower_components/DOMPurify/dist/purify.min.js', included: false}, + {pattern: 'app/bower_components/he/he.js', included: false}, {pattern: 'app/bower_components/flight/**/*.js', included: false}, {pattern: 'app/bower_components/i18next/**/*.js', included: false}, {pattern: 'app/bower_components/quoted-printable/*.js', included: false}, diff --git a/web-ui/test/spec/helpers/sanitizer.spec.js b/web-ui/test/spec/helpers/sanitizer.spec.js new file mode 100644 index 00000000..acd4b2b2 --- /dev/null +++ b/web-ui/test/spec/helpers/sanitizer.spec.js @@ -0,0 +1,49 @@ +define(['helpers/sanitizer'], function (sanitizer) { + 'use strict'; + + describe('sanitizer', function () { + + describe('sanitizer.addLineBreaks', function () { + it('should add line breaks', function () { + var expectedOutput = 'foo
bar'; + var output = sanitizer.addLineBreaks('foo\nbar'); + expect(output).toEqual(expectedOutput); + }); + }); + + describe('sanitizer.purifyHtml', function () { + it('should fire up DOMPurify', function () { + var expectedOutput = '123I am a dolphin!'; + var output = sanitizer.purifyHtml('123I am a dolphin!'); + expect(output).toEqual(expectedOutput); + }); + }); + + describe('sanitizer.purifyText', function () { + it('should escape HTML', function () { + var expectedOutput = '123<a>asd</a>'; + var output = sanitizer.purifyText('123asd'); + expect(output).toEqual(expectedOutput); + }); + }); + + describe('sanitizer.sanitize', function () { + it('should sanitize a plaintext mail', function () { + var expectedOutput = '123<a>asd</a>'; + var output = sanitizer.sanitize({ + textPlainBody: '123asd' + }); + expect(output).toEqual(expectedOutput); + }); + + it('should sanitize an html mail', function () { + var expectedOutput = '
123I am a dolphin!foobar
'; + var output = sanitizer.sanitize({ + htmlBody: '
123I am a dolphin!foobar
' + }); + expect(output).toEqual(expectedOutput); + }); + }); + + }); +}); diff --git a/web-ui/test/spec/helpers/view_helper.spec.js b/web-ui/test/spec/helpers/view_helper.spec.js index 92a31a1f..b2f597c2 100644 --- a/web-ui/test/spec/helpers/view_helper.spec.js +++ b/web-ui/test/spec/helpers/view_helper.spec.js @@ -90,13 +90,6 @@ define(['helpers/view_helper'], function (viewHelper) { }); }); - it('each line of plain text mail gets a new paragraph', function () { - var formattedMail = $('
'); - formattedMail.html(viewHelper.formatMailBody(testData.parsedMail.simpleTextPlain)); - expect(formattedMail).toContainHtml('
Hello Everyone
'); - }); - - it('escape html in plain text body', function () { var formattedMail = $('
'); var mail = testData.parsedMail.simpleTextPlain; diff --git a/web-ui/test/test-main.js b/web-ui/test/test-main.js index 7d87d9de..17ba3876 100644 --- a/web-ui/test/test-main.js +++ b/web-ui/test/test-main.js @@ -14,6 +14,8 @@ requirejs.config({ 'lib': 'app/js/lib', 'hbs': 'app/js/generated/hbs', 'flight': 'app/bower_components/flight', + 'DOMPurify': 'app/bower_components/DOMPurify/dist/purify.min', + 'he': 'app/bower_components/he/he', 'views': 'app/js/views', 'helpers': 'app/js/helpers', 'feedback': 'app/js/feedback', @@ -35,7 +37,6 @@ requirejs.config({ 'user_settings': 'app/js/user_settings' }, - deps: tests, callback: function () { -- cgit v1.2.3