From 4bf7f863dc2ffea1ea105e575205ab0f83ed2751 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 | 59 ++++++++++++++++++++++++++++++------- js/test/Alert.js | 28 ------------------ js/test/AttachmentViewer.js | 20 ++++++++----- js/test/Check.js | 3 -- js/test/DiscussionViewer.js | 3 -- js/test/Editor.js | 3 -- js/test/Helper.js | 42 +++++++++++--------------- js/test/I18n.js | 3 +- js/test/Model.js | 27 ++++++++--------- js/test/PasteStatus.js | 7 ----- js/test/PasteViewer.js | 3 -- js/test/Prompt.js | 5 +--- tpl/bootstrap.php | 2 +- tpl/page.php | 2 +- 15 files changed, 95 insertions(+), 143 deletions(-) diff --git a/js/common.js b/js/common.js index a33dfe1..c60bf75 100644 --- a/js/common.js +++ b/js/common.js @@ -40,21 +40,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 = { - '&': '&', - '<': '<', - '>': '>', - '"': '"', - "'": ''', - '/': '/', - '`': '`', - '=': '=' - }, mimeFile = fs.createReadStream('/etc/mime.types'), mimeLine = ''; @@ -97,22 +82,6 @@ function parseMime(line) { exports.atob = atob; exports.btoa = btoa; -/** - * 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 dc7adef..66cc98e 100644 --- a/js/privatebin.js +++ b/js/privatebin.js @@ -440,7 +440,33 @@ jQuery.PrivateBin = (function($, RawDeflate) { expirationDate = expirationDate.setUTCSeconds(expirationDate.getUTCSeconds() + secondsToExpiration); return expirationDate; - } + }; + + /** + * 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; })(); @@ -592,16 +618,31 @@ jQuery.PrivateBin = (function($, 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 let output = Helper.sprintf.apply(this, args); // if $element is given, apply text to element if ($element !== null) { - // avoid HTML entity encoding if translation contains link - if (output.indexOf('').text(text).html() + Helper.htmlEntities(text) ), sanitizedLinkedText = DOMPurify.sanitize(escapedLinkedText); $plainText.html(sanitizedLinkedText); @@ -2796,11 +2837,7 @@ jQuery.PrivateBin = (function($, RawDeflate) { $attachmentLink.appendTo($element); // update text - ensuring no HTML is inserted into the text node - I18n._( - $attachmentLink, - $('
').text(label).html(), - $('
').text($attachmentLink.attr('download')).html() - ); + I18n._($attachmentLink, label, $attachmentLink.attr('download')); }; /** @@ -3498,7 +3535,7 @@ jQuery.PrivateBin = (function($, RawDeflate) { for (let i = 0; i < $head.length; ++i) { newDoc.write($head[i].outerHTML); } - newDoc.write('
' + DOMPurify.sanitize($('
').text(paste).html()) + '
'); + newDoc.write('
' + DOMPurify.sanitize(Helper.htmlEntities(paste)) + '
'); newDoc.close(); } diff --git a/js/test/Alert.js b/js/test/Alert.js index d59f38c..a6c888f 100644 --- a/js/test/Alert.js +++ b/js/test/Alert.js @@ -67,10 +67,6 @@ describe('Alert', function () { }); describe('showWarning', function () { - before(function () { - cleanup(); - }); - jsc.property( 'shows a warning message (basic)', jsc.array(common.jscAlnumString()), @@ -136,10 +132,6 @@ describe('Alert', function () { }); describe('showError', function () { - before(function () { - cleanup(); - }); - jsc.property( 'shows an error message (basic)', jsc.array(common.jscAlnumString()), @@ -205,10 +197,6 @@ describe('Alert', function () { }); describe('showRemaining', function () { - before(function () { - cleanup(); - }); - jsc.property( 'shows remaining time (basic)', jsc.array(common.jscAlnumString()), @@ -254,10 +242,6 @@ describe('Alert', function () { }); describe('showLoading', function () { - before(function () { - cleanup(); - }); - jsc.property( 'shows a loading message (basic)', jsc.array(common.jscAlnumString()), @@ -310,10 +294,6 @@ describe('Alert', function () { }); describe('hideLoading', function () { - before(function () { - cleanup(); - }); - it( 'hides the loading message', function() { @@ -335,10 +315,6 @@ describe('Alert', function () { }); describe('hideMessages', function () { - before(function () { - cleanup(); - }); - it( 'hides all messages', function() { @@ -361,10 +337,6 @@ describe('Alert', function () { }); describe('setCustomHandler', function () { - before(function () { - cleanup(); - }); - jsc.property( 'calls a given handler function', 'nat 3', diff --git a/js/test/AttachmentViewer.js b/js/test/AttachmentViewer.js index e891deb..f3b3bb5 100644 --- a/js/test/AttachmentViewer.js +++ b/js/test/AttachmentViewer.js @@ -4,9 +4,6 @@ var common = require('../common'); describe('AttachmentViewer', function () { describe('setAttachment, showAttachment, removeAttachment, hideAttachment, hideAttachmentPreview, hasAttachment, getAttachment & moveAttachmentTo', function () { this.timeout(30000); - before(function () { - cleanup(); - }); jsc.property( 'displays & hides data as requested', @@ -16,7 +13,7 @@ describe('AttachmentViewer', function () { 'string', 'string', function (mimeType, rawdata, filename, prefix, postfix) { - var clean = jsdom(), + let clean = jsdom(), data = 'data:' + mimeType + ';base64,' + btoa(rawdata), previewSupported = ( mimeType.substring(0, 6) === 'image/' || @@ -24,7 +21,8 @@ describe('AttachmentViewer', function () { mimeType.substring(0, 6) === 'video/' || mimeType.match(/\/pdf/i) ), - results = []; + results = [], + result = ''; prefix = prefix.replace(/%(s|d)/g, '%%'); postfix = postfix.replace(/%(s|d)/g, '%%'); $('body').html( @@ -57,7 +55,7 @@ describe('AttachmentViewer', function () { } // beyond this point we will get the blob URL instead of the data data = window.URL.createObjectURL(data); - var attachment = $.PrivateBin.AttachmentViewer.getAttachment(); + const attachment = $.PrivateBin.AttachmentViewer.getAttachment(); results.push( $.PrivateBin.AttachmentViewer.hasAttachment() && $('#attachment').hasClass('hidden') && @@ -84,13 +82,19 @@ describe('AttachmentViewer', function () { !$('#attachment').hasClass('hidden') && (previewSupported ? !$('#attachmentPreview').hasClass('hidden') : $('#attachmentPreview').hasClass('hidden')) ); - var element = $('
'); + let element = $('
'); $.PrivateBin.AttachmentViewer.moveAttachmentTo(element, prefix + '%s' + postfix); + // messageIDs with links get a relaxed treatment + if (prefix.indexOf('').html(prefix + $.PrivateBin.Helper.htmlEntities(filename) + postfix).html(); + } if (filename.length) { results.push( element.children()[0].href === data && element.children()[0].getAttribute('download') === filename && - element.children()[0].text === $('
').text(prefix + filename + postfix).html() + element.children()[0].text === result ); } else { results.push(element.children()[0].href === data); diff --git a/js/test/Check.js b/js/test/Check.js index bf4299b..474330e 100644 --- a/js/test/Check.js +++ b/js/test/Check.js @@ -5,9 +5,6 @@ var common = require('../common'); describe('Check', function () { describe('init', function () { this.timeout(30000); - before(function () { - cleanup(); - }); it('returns false and shows error, if a bot UA is detected', function () { jsc.assert(jsc.forall( diff --git a/js/test/DiscussionViewer.js b/js/test/DiscussionViewer.js index 3095d8e..09697f3 100644 --- a/js/test/DiscussionViewer.js +++ b/js/test/DiscussionViewer.js @@ -4,9 +4,6 @@ var common = require('../common'); describe('DiscussionViewer', function () { describe('handleNotification, prepareNewDiscussion, addComment, finishDiscussion, getReplyMessage, getReplyNickname, getReplyCommentId & highlightComment', function () { this.timeout(30000); - before(function () { - cleanup(); - }); jsc.property( 'displays & hides comments as requested', diff --git a/js/test/Editor.js b/js/test/Editor.js index 86a9d9f..3f86257 100644 --- a/js/test/Editor.js +++ b/js/test/Editor.js @@ -4,9 +4,6 @@ require('../common'); describe('Editor', function () { describe('show, hide, getText, setText & isPreview', function () { this.timeout(30000); - before(function () { - cleanup(); - }); jsc.property( 'returns text fed into the textarea, handles editor tabs', diff --git a/js/test/Helper.js b/js/test/Helper.js index 3002d2a..4399089 100644 --- a/js/test/Helper.js +++ b/js/test/Helper.js @@ -3,10 +3,6 @@ var common = require('../common'); describe('Helper', function () { describe('secondsToHuman', function () { - after(function () { - cleanup(); - }); - jsc.property('returns an array with a number and a word', 'integer', function (number) { var result = $.PrivateBin.Helper.secondsToHuman(number); return Array.isArray(result) && @@ -57,11 +53,11 @@ describe('Helper', function () { 'nearray string', function (ids, contents) { var html = '', - result = true; + result = true, + clean = jsdom(html); ids.forEach(function(item, i) { - html += '
' + common.htmlEntities(contents[i] || contents[0]) + '
'; + html += '
' + $.PrivateBin.Helper.htmlEntities(contents[i] || contents[0]) + '
'; }); - var clean = jsdom(html); // TODO: As per https://github.com/tmpvar/jsdom/issues/321 there is no getSelection in jsdom, yet. // Once there is one, uncomment the block below to actually check the result. /* @@ -77,8 +73,8 @@ describe('Helper', function () { }); describe('urls2links', function () { - after(function () { - cleanup(); + before(function () { + cleanup = jsdom(); }); jsc.property( @@ -97,11 +93,11 @@ describe('Helper', function () { jsc.array(common.jscHashString()), 'string', function (prefix, schema, address, query, fragment, postfix) { - var query = query.join(''), + var query = query.join(''), fragment = fragment.join(''), - url = schema + '://' + address.join('') + '/?' + query + '#' + fragment, - prefix = common.htmlEntities(prefix), - postfix = ' ' + common.htmlEntities(postfix); + url = schema + '://' + address.join('') + '/?' + query + '#' + fragment, + prefix = $.PrivateBin.Helper.htmlEntities(prefix), + postfix = ' ' + $.PrivateBin.Helper.htmlEntities(postfix); // special cases: When the query string and fragment imply the beginning of an HTML entity, eg. � or &#x if ( @@ -122,19 +118,15 @@ describe('Helper', function () { jsc.array(common.jscQueryString()), 'string', function (prefix, query, postfix) { - var url = 'magnet:?' + query.join('').replace(/^&+|&+$/gm,''), - prefix = common.htmlEntities(prefix), - postfix = common.htmlEntities(postfix); + var url = 'magnet:?' + query.join('').replace(/^&+|&+$/gm,''), + prefix = $.PrivateBin.Helper.htmlEntities(prefix), + postfix = $.PrivateBin.Helper.htmlEntities(postfix); return prefix + '' + url + ' ' + postfix === $.PrivateBin.Helper.urls2links(prefix + url + ' ' + postfix); } ); }); describe('sprintf', function () { - after(function () { - cleanup(); - }); - jsc.property( 'replaces %s in strings with first given parameter', 'string', @@ -211,7 +203,7 @@ describe('Helper', function () { describe('getCookie', function () { this.timeout(30000); - after(function () { + before(function () { cleanup(); }); @@ -263,16 +255,16 @@ describe('Helper', function () { }); describe('htmlEntities', function () { - after(function () { - cleanup(); + before(function () { + cleanup = jsdom(); }); jsc.property( 'removes all HTML entities from any given string', 'string', function (string) { - var result = common.htmlEntities(string); - return !(/[<>"'`=\/]/.test(result)) && !(string.indexOf('&') > -1 && !(/&/.test(result))); + var result = $.PrivateBin.Helper.htmlEntities(string); + return !(/[<>]/.test(result)) && !(string.indexOf('&') > -1 && !(/&/.test(result))); } ); }); diff --git a/js/test/I18n.js b/js/test/I18n.js index 06c65b7..9bfc76a 100644 --- a/js/test/I18n.js +++ b/js/test/I18n.js @@ -32,6 +32,7 @@ describe('I18n', function () { var fakeAlias = $.PrivateBin.I18n._(fake); $.PrivateBin.I18n.reset(); + messageId = $.PrivateBin.Helper.htmlEntities(messageId); return messageId === result && messageId === alias && messageId === pluralResult && messageId === pluralAlias && messageId === fakeResult && messageId === fakeAlias; @@ -46,7 +47,7 @@ describe('I18n', function () { prefix = prefix.replace(/%(s|d)/g, '%%'); params[0] = params[0].replace(/%(s|d)/g, '%%'); postfix = postfix.replace(/%(s|d)/g, '%%'); - var translation = prefix + params[0] + postfix; + var translation = $.PrivateBin.Helper.htmlEntities(prefix + params[0] + postfix); params.unshift(prefix + '%s' + postfix); var result = $.PrivateBin.I18n.translate.apply(this, params); $.PrivateBin.I18n.reset(); diff --git a/js/test/Model.js b/js/test/Model.js index cfcd6db..9ebc547 100644 --- a/js/test/Model.js +++ b/js/test/Model.js @@ -5,18 +5,18 @@ describe('Model', function () { describe('getExpirationDefault', function () { before(function () { $.PrivateBin.Model.reset(); - cleanup(); + cleanup = jsdom(); }); jsc.property( 'returns the contents of the element with id "pasteExpiration"', - 'array asciinestring', + 'nearray asciinestring', 'string', 'small nat', function (keys, value, key) { - keys = keys.map(common.htmlEntities); - value = common.htmlEntities(value); - var content = keys.length > key ? keys[key] : (keys.length > 0 ? keys[0] : 'null'), + keys = keys.map($.PrivateBin.Helper.htmlEntities); + value = $.PrivateBin.Helper.htmlEntities(value); + var content = keys.length > key ? keys[key] : keys[0], contents = ''; keys.forEach(function(item) { contents += '
' + '
' ); + $.PrivateBin.Model.reset(); $.PrivateBin.Model.init(); $.PrivateBin.Prompt.init(); $.PrivateBin.Prompt.requestPassword(); diff --git a/tpl/bootstrap.php b/tpl/bootstrap.php index 50e3e25..a210a85 100644 --- a/tpl/bootstrap.php +++ b/tpl/bootstrap.php @@ -72,7 +72,7 @@ endif; ?> - + diff --git a/tpl/page.php b/tpl/page.php index dcb939b..8aac885 100644 --- a/tpl/page.php +++ b/tpl/page.php @@ -50,7 +50,7 @@ endif; ?> - +