From 6ccbad612dce6fad358178e15a0764fbf0709735 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Sun, 16 Feb 2020 08:37:33 +0100 Subject: [PATCH] backporting double encoding fixes from #560 --- CHANGELOG.md | 2 + js/common.js | 2 +- js/privatebin.js | 171 ++++++++++++++++++++---------------- js/test/AttachmentViewer.js | 21 +++-- js/test/Helper.js | 52 ++++++----- js/test/I18n.js | 117 ++++++++++++++++++++++-- js/test/PasteStatus.js | 5 +- tpl/bootstrap.php | 2 +- tpl/page.php | 2 +- 9 files changed, 249 insertions(+), 125 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 82df4952..33ba0659 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # PrivateBin version history + * **1.2.3 (2020-02-16)** + * FIXED: HTML entity double encoding issues introduced in 1.3.2 (#560) * **1.2.2 (2020-01-11)** * CHANGED: Upgrading libraries to: bootstrap 3.4.1, DOMpurify 2.0.7, jQuery 3.4.1, kjua 0.6.0, Showdown 1.9.1 & SJCL 1.0.8 * FIXED: HTML injection via unescaped attachment filename (#554) diff --git a/js/common.js b/js/common.js index 4c77eb8e..3ef77fab 100644 --- a/js/common.js +++ b/js/common.js @@ -32,7 +32,7 @@ var a2zString = ['a','b','c','d','e','f','g','h','i','j','k','l','m', return c.toUpperCase(); }) ), - schemas = ['ftp','gopher','http','https','ws','wss'], + schemas = ['ftp','http','https'], supportedLanguages = ['de', 'es', 'fr', 'it', 'no', 'pl', 'pt', 'oc', 'ru', 'sl', 'zh'], mimeTypes = ['image/png', 'application/octet-stream'], formats = ['plaintext', 'markdown', 'syntaxhighlighting'], diff --git a/js/privatebin.js b/js/privatebin.js index e0babafd..20babe93 100644 --- a/js/privatebin.js +++ b/js/privatebin.js @@ -68,6 +68,26 @@ jQuery.PrivateBin = (function($, sjcl, Base64, RawDeflate) { */ var baseUri = null; + /** + * character to HTML entity lookup table + * + * @see {@link https://github.com/janl/mustache.js/blob/master/mustache.js#L60} + * @name Helper.entityMap + * @private + * @enum {Object} + * @readonly + */ + var entityMap = { + '&': '&', + '<': '<', + '>': '>', + '"': '"', + "'": ''', + '/': '/', + '`': '`', + '=': '=' + }; + /** * converts a duration (in seconds) into human friendly approximation * @@ -171,19 +191,12 @@ jQuery.PrivateBin = (function($, sjcl, Base64, RawDeflate) { var format = args[0], i = 1; return format.replace(/%(s|d)/g, function (m) { - // m is the matched format, e.g. %s, %d var val = args[i]; - // A switch statement so that the formatter can be extended. - switch (m) - { - case '%d': - val = parseFloat(val); - if (isNaN(val)) { - val = 0; - } - break; - default: - // Default is %s + if (m === '%d') { + val = parseFloat(val); + if (isNaN(val)) { + val = 0; + } } ++i; return val; @@ -237,15 +250,21 @@ jQuery.PrivateBin = (function($, sjcl, Base64, RawDeflate) { }; /** - * resets state, used for unit testing + * convert all applicable characters to HTML entities * - * @name Helper.reset + * @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.reset = function() - { - baseUri = null; - }; + me.htmlEntities = function(str) { + return String(str).replace( + /[&<>"'`=\/]/g, function(s) { + return entityMap[s]; + } + ); + } /** * checks whether this is a bot we dislike @@ -268,29 +287,14 @@ jQuery.PrivateBin = (function($, sjcl, Base64, RawDeflate) { } /** - * encode all applicable characters to HTML entities + * resets state, used for unit testing * - * @see {@link https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html} - * - * @name Helper.htmlEntities + * @name Helper.reset * @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]; - }); + me.reset = function() + { + baseUri = null; }; return me; @@ -363,10 +367,14 @@ jQuery.PrivateBin = (function($, sjcl, Base64, RawDeflate) { * * Optionally pass a jQuery element as the first parameter, to automatically * let the text of this element be replaced. In case the (asynchronously - * loaded) language is not downloadet yet, this will make sure the string - * is replaced when it is actually loaded. - * So for easy translations passing the jQuery object to apply it to is - * more save, especially when they are loaded in the beginning. + * loaded) language is not downloaded yet, this will make sure the string + * is replaced when it eventually gets loaded. Using this is both simpler + * and more secure, as it avoids potential XSS when inserting text. + * The next parameter is the message ID, matching the ones found in + * the translation files under the i18n directory. + * Any additional parameters will get inserted into the message ID in + * place of %s (strings) or %d (digits), applying the appropriate plural + * in case of digits. See also Helper.sprintf(). * * @name I18n.translate * @function @@ -446,31 +454,39 @@ jQuery.PrivateBin = (function($, sjcl, Base64, RawDeflate) { } // 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]); + var containsLinks = args[0].indexOf(' 0) may never contain HTML as they may come from untrusted parties + if ((containsLinks ? i > 1 : i > 0) || !containsLinks) { + args[i] = Helper.htmlEntities(args[i]); + } } } - // format string var output = Helper.sprintf.apply(this, args); - // if $element is given, apply text to element + if (containsLinks) { + // only allow tags/attributes we actually use in translations + output = DOMPurify.sanitize( + output, { + ALLOWED_TAGS: ['a', 'i', 'span'], + ALLOWED_ATTR: ['href', 'id'] + } + ); + } + + // if $element is given, insert translation if ($element !== null) { - if (containsNoLinks) { - // avoid HTML entity encoding if translation contains links - $element.text(output); + if (containsLinks) { + $element.html(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'] - }) - ); + // text node takes care of entity encoding + $element.text(output); } + return ''; } return output; @@ -1342,11 +1358,10 @@ jQuery.PrivateBin = (function($, sjcl, Base64, RawDeflate) { */ me.createPasteNotification = function(url, deleteUrl) { - $('#pastelink').html( - I18n._( - 'Your paste is %s (Hit [Ctrl]+[c] to copy)', - url, url - ) + I18n._( + $('#pastelink'), + 'Your paste is %s (Hit [Ctrl]+[c] to copy)', + url, url ); // save newly created element $pasteUrl = $('#pasteurl'); @@ -1354,7 +1369,8 @@ jQuery.PrivateBin = (function($, sjcl, Base64, RawDeflate) { $pasteUrl.click(pasteLinkClick); // shorten button - $('#deletelink').html('' + I18n._('Delete data') + ''); + $('#deletelink').html(''); + I18n._($('#deletelink a').first(), 'Delete data'); // show result $pasteSuccess.removeClass('hidden'); @@ -1810,10 +1826,13 @@ jQuery.PrivateBin = (function($, sjcl, Base64, RawDeflate) { } // escape HTML entities, link URLs, sanitize - var escapedLinkedText = Helper.urls2links( - Helper.htmlEntities(text) - ), - sanitizedLinkedText = DOMPurify.sanitize(escapedLinkedText); + var escapedLinkedText = Helper.urls2links(text), + sanitizedLinkedText = DOMPurify.sanitize( + escapedLinkedText, { + ALLOWED_TAGS: ['a'], + ALLOWED_ATTR: ['href', 'rel'] + } + ); $plainText.html(sanitizedLinkedText); $prettyPrint.html(sanitizedLinkedText); @@ -2625,7 +2644,10 @@ jQuery.PrivateBin = (function($, sjcl, Base64, RawDeflate) { // set & parse text $commentEntryData.html( DOMPurify.sanitize( - Helper.urls2links(commentText) + Helper.urls2links(commentText), { + ALLOWED_TAGS: ['a'], + ALLOWED_ATTR: ['href', 'rel'] + } ) ); @@ -4418,9 +4440,7 @@ jQuery.PrivateBin = (function($, sjcl, Base64, RawDeflate) { Uploader.setUnencryptedData('deletetoken', deleteToken); Uploader.setFailure(function () { - Alert.showError( - I18n._('Could not delete the paste, it was not stored in burn after reading mode.') - ); + Alert.showError('Could not delete the paste, it was not stored in burn after reading mode.'); }); Uploader.run(); }; @@ -4436,7 +4456,10 @@ jQuery.PrivateBin = (function($, sjcl, Base64, RawDeflate) { // first load translations I18n.loadTranslations(); - DOMPurify.setConfig({SAFE_FOR_JQUERY: true}); + DOMPurify.setConfig({ + ALLOWED_URI_REGEXP: /^(?:(?:(?:f|ht)tps?|mailto|magnet):)/i, + SAFE_FOR_JQUERY: true + }); // initialize other modules/"classes" Alert.init(); diff --git a/js/test/AttachmentViewer.js b/js/test/AttachmentViewer.js index b755da07..2c129153 100644 --- a/js/test/AttachmentViewer.js +++ b/js/test/AttachmentViewer.js @@ -8,13 +8,13 @@ describe('AttachmentViewer', function () { jsc.property( 'displays & hides data as requested', common.jscMimeTypes(), - jsc.nearray(common.jscBase64String()), 'string', 'string', 'string', - function (mimeType, base64, filename, prefix, postfix) { - var clean = jsdom(), - data = 'data:' + mimeType + ';base64,' + base64.join(''), + 'string', + function (mimeType, rawdata, filename, prefix, postfix) { + let clean = jsdom(), + data = 'data:' + mimeType + ';base64,' + btoa(rawdata), previewSupported = ( mimeType.substring(0, 6) === 'image/' || mimeType.substring(0, 6) === 'audio/' || @@ -23,7 +23,7 @@ describe('AttachmentViewer', function () { ), results = [], result = ''; - prefix = prefix.replace(/%(s|d)/g, '%%'); + prefix = prefix.replace(/%(s|d)/g, '%%'); postfix = postfix.replace(/%(s|d)/g, '%%'); $('body').html( '