From 2caddf985f35c6b660b19ff62bb9ddd2d9f33118 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Sat, 4 Jan 2020 11:34:16 +0100 Subject: [PATCH] more general solution addressing #554, kudos @rugk for the suggestions --- js/common.js | 31 ------- js/privatebin.js | 91 ++++++++++++++++----- js/test/Alert.js | 156 ++++++++++++++++++++++++++++-------- js/test/AttachmentViewer.js | 16 ++-- js/test/DiscussionViewer.js | 3 - js/test/Editor.js | 3 - js/test/Helper.js | 44 +++++----- js/test/I18n.js | 3 +- js/test/Model.js | 26 +++--- js/test/PasteStatus.js | 7 -- js/test/PasteViewer.js | 3 - js/test/Prompt.js | 5 +- tpl/bootstrap.php | 2 +- tpl/page.php | 2 +- 14 files changed, 241 insertions(+), 151 deletions(-) diff --git a/js/common.js b/js/common.js index 187fb05d..12be3cc5 100644 --- a/js/common.js +++ b/js/common.js @@ -36,21 +36,6 @@ var a2zString = ['a','b','c','d','e','f','g','h','i','j','k','l','m', supportedLanguages = ['de', 'es', 'fr', 'it', 'no', 'pl', 'pt', 'oc', 'ru', 'sl', 'zh'], mimeTypes = ['image/png', 'application/octet-stream'], formats = ['plaintext', 'markdown', 'syntaxhighlighting'], - /** - * character to HTML entity lookup table - * - * @see {@link https://github.com/janl/mustache.js/blob/master/mustache.js#L60} - */ - entityMap = { - '&': '&', - '<': '<', - '>': '>', - '"': '"', - "'": ''', - '/': '/', - '`': '`', - '=': '=' - }, logFile = fs.createWriteStream('test.log'), mimeFile = fs.createReadStream('/etc/mime.types'), mimeLine = ''; @@ -97,22 +82,6 @@ function parseMime(line) { // common testing helper functions -/** - * convert all applicable characters to HTML entities - * - * @see {@link https://www.owasp.org/index.php/XSS_(Cross_Site_Scripting)_Prevention_Cheat_Sheet#RULE_.231_-_HTML_Escape_Before_Inserting_Untrusted_Data_into_HTML_Element_Content} - * @name htmlEntities - * @function - * @param {string} str - * @return {string} escaped HTML - */ -exports.htmlEntities = function(str) { - return String(str).replace( - /[&<>"'`=\/]/g, function(s) { - return entityMap[s]; - }); -}; - // provides random lowercase characters from a to z exports.jscA2zString = function() { return jsc.elements(a2zString); diff --git a/js/privatebin.js b/js/privatebin.js index be660438..b2608ca4 100644 --- a/js/privatebin.js +++ b/js/privatebin.js @@ -267,6 +267,32 @@ jQuery.PrivateBin = (function($, sjcl, Base64, RawDeflate) { return false; } + /** + * encode all applicable characters to HTML entities + * + * @see {@link https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html} + * + * @name Helper.htmlEntities + * @function + * @param string str + * @return string escaped HTML + */ + me.htmlEntities = function(str) { + // using textarea, since other tags may allow and execute scripts, even when detached from DOM + let holder = document.createElement('textarea'); + holder.textContent = str; + // as per OWASP recommendation, also encoding quotes and slash + return holder.innerHTML.replace( + /["'\/]/g, + function(s) { + return { + '"': '"', + "'": ''', + '/': '/' + }[s]; + }); + }; + return me; })(); @@ -419,17 +445,31 @@ jQuery.PrivateBin = (function($, sjcl, Base64, RawDeflate) { args[0] = translations[messageId]; } + // messageID may contain links, but should be from a trusted source (code or translation JSON files) + let containsNoLinks = args[0].indexOf(' 0) may never contain HTML as they may come from untrusted parties + if (i > 0 || containsNoLinks) { + args[i] = Helper.htmlEntities(args[i]); + } + } + // format string var output = Helper.sprintf.apply(this, args); // if $element is given, apply text to element if ($element !== null) { - // get last text node of element - var content = $element.contents(); - if (content.length > 1) { - content[content.length - 1].nodeValue = ' ' + output; - } else { + if (containsNoLinks) { + // avoid HTML entity encoding if translation contains links $element.text(output); + } else { + // only allow tags/attributes we actually use in our translations + $element.html( + DOMPurify.sanitize(output, { + ALLOWED_TAGS: ['a', 'br', 'i', 'span'], + ALLOWED_ATTR: ['href', 'id'] + }) + ); } } @@ -1052,28 +1092,35 @@ jQuery.PrivateBin = (function($, sjcl, Base64, RawDeflate) { icon = null; // icons not supported in this case } } + var $translationTarget = $element; - // handle icon - if (icon !== null && // icon was passed - icon !== currentIcon[id] // and it differs from current icon - ) { - var $glyphIcon = $element.find(':first'); + // handle icon, if template uses one + var $glyphIcon = $element.find(':first'); + if ($glyphIcon.length) { + // if there is an icon, we need to provide an inner element + // to translate the message into, instead of the parent + $translationTarget = $(''); + $element.html(' ').prepend($glyphIcon).append($translationTarget); - // remove (previous) icon - $glyphIcon.removeClass(currentIcon[id]); + if (icon !== null && // icon was passed + icon !== currentIcon[id] // and it differs from current icon + ) { + // remove (previous) icon + $glyphIcon.removeClass(currentIcon[id]); - // any other thing as a string (e.g. 'null') (only) removes the icon - if (typeof icon === 'string') { - // set new icon - currentIcon[id] = 'glyphicon-' + icon; - $glyphIcon.addClass(currentIcon[id]); + // any other thing as a string (e.g. 'null') (only) removes the icon + if (typeof icon === 'string') { + // set new icon + currentIcon[id] = 'glyphicon-' + icon; + $glyphIcon.addClass(currentIcon[id]); + } } } // show text if (args !== null) { // add jQuery object to it as first parameter - args.unshift($element); + args.unshift($translationTarget); // pass it to I18n I18n._.apply(this, args); } @@ -1764,9 +1811,9 @@ jQuery.PrivateBin = (function($, sjcl, Base64, RawDeflate) { // escape HTML entities, link URLs, sanitize var escapedLinkedText = Helper.urls2links( - $('
').text(text).html() - ), - sanitizedLinkedText = DOMPurify.sanitize(escapedLinkedText); + Helper.htmlEntities(text) + ), + sanitizedLinkedText = DOMPurify.sanitize(escapedLinkedText); $plainText.html(sanitizedLinkedText); $prettyPrint.html(sanitizedLinkedText); @@ -2894,7 +2941,7 @@ jQuery.PrivateBin = (function($, sjcl, Base64, RawDeflate) { for (var i = 0; i < $head.length; i++) { newDoc.write($head[i].outerHTML); } - newDoc.write('
' + DOMPurify.sanitize(paste) + '
'); + newDoc.write('
' + DOMPurify.sanitize(Helper.htmlEntities(paste)) + '
'); newDoc.close(); } diff --git a/js/test/Alert.js b/js/test/Alert.js index 617c4889..324380f5 100644 --- a/js/test/Alert.js +++ b/js/test/Alert.js @@ -3,21 +3,56 @@ var common = require('../common'); describe('Alert', function () { describe('showStatus', function () { - before(function () { - cleanup(); - }); - jsc.property( 'shows a status message', jsc.array(common.jscAlnumString()), jsc.array(common.jscAlnumString()), + function (icon, message) { + icon = icon.join(''); + message = message.join(''); + var expected = '
' + message + '
'; + $('body').html( + '
' + ); + $.PrivateBin.Alert.init(); + $.PrivateBin.Alert.showStatus(message, icon); + var result = $('body').html(); + return expected === result; + } + ); + + jsc.property( + 'shows a status message (bootstrap)', + jsc.array(common.jscAlnumString()), + function (message) { + message = message.join(''); + var expected = ''; + $('body').html( + '' + ); + $.PrivateBin.Alert.init(); + $.PrivateBin.Alert.showStatus(message); + var result = $('body').html(); + return expected === result; + } + ); + + jsc.property( + 'shows a status message (bootstrap, custom icon)', + jsc.array(common.jscAlnumString()), + jsc.array(common.jscAlnumString()), function (icon, message) { icon = icon.join(''); message = message.join(''); var expected = ''; + '" aria-hidden="true"> ' + message + '
'; $('body').html( ''; + $('body').html( + '
' + ); + $.PrivateBin.Alert.init(); + $.PrivateBin.Alert.showError(message, icon); + var result = $('body').html(); + return expected === result; + } + ); jsc.property( - 'shows an error message', + 'shows an error message (bootstrap)', + jsc.array(common.jscAlnumString()), + jsc.array(common.jscAlnumString()), + function (icon, message) { + message = message.join(''); + var expected = ''; + $('body').html( + '' + ); + $.PrivateBin.Alert.init(); + $.PrivateBin.Alert.showError(message); + var result = $('body').html(); + return expected === result; + } + ); + + jsc.property( + 'shows an error message (bootstrap, custom icon)', jsc.array(common.jscAlnumString()), jsc.array(common.jscAlnumString()), function (icon, message) { @@ -46,7 +117,7 @@ describe('Alert', function () { var expected = ''; + '" aria-hidden="true"> ' + message + ''; $('body').html( ''; + $('body').html( + '' + ); + $.PrivateBin.Alert.init(); + $.PrivateBin.Alert.showRemaining(['%s' + message + '%d', string, number]); + var result = $('body').html(); + return expected === result; + } + ); jsc.property( - 'shows remaining time', + 'shows remaining time (bootstrap)', jsc.array(common.jscAlnumString()), jsc.array(common.jscAlnumString()), 'integer', @@ -76,7 +162,7 @@ describe('Alert', function () { var expected = ''; + ' ' + string + message + number + ''; $('body').html( ''; + $('body').html( + '' + ); + $.PrivateBin.Alert.init(); + $.PrivateBin.Alert.showLoading(message, icon); + var result = $('body').html(); + return expected === result; + } + ); jsc.property( - 'shows a loading message', + 'shows a loading message (bootstrap)', jsc.array(common.jscAlnumString()), jsc.array(common.jscAlnumString()), function (message, icon) { @@ -109,7 +213,7 @@ describe('Alert', function () { var expected = ''; + '" aria-hidden="true"> ' + message + ''; $('body').html( '