From 56f4ee5c20fc6dfd22c86c3f84337ff941a93e4f Mon Sep 17 00:00:00 2001 From: rugk Date: Wed, 22 Nov 2017 16:48:54 +0100 Subject: [PATCH] Revert "Try to move sanitisation & links into setElementText" This reverts commit 8d2e19f7917231dee1af2453f55ddbd21fd83af5. --- js/privatebin.js | 139 ++++++++++++++++++++++++++--------------------- js/test.js | 40 +++++++------- 2 files changed, 96 insertions(+), 83 deletions(-) diff --git a/js/privatebin.js b/js/privatebin.js index df5dffc..86b6046 100644 --- a/js/privatebin.js +++ b/js/privatebin.js @@ -44,43 +44,33 @@ jQuery.PrivateBin = function($, sjcl, Base64, RawDeflate) { var me = {}; /** - * cache for script location + * character to HTML entity lookup table * - * @name Helper.baseUri + * @see {@link https://github.com/janl/mustache.js/blob/master/mustache.js#L60} + * @name Helper.entityMap * @private - * @enum {string|null} + * @enum {Object} + * @readonly */ - var baseUri = null; + var entityMap = { + '&': '&', + '<': '<', + '>': '>', + '"': '"', + "'": ''', + '/': '/', + '`': '`', + '=': '=' + }; /** - * convert URLs to clickable links. - * URLs to handle: - * 
-         *     magnet:?xt.1=urn:sha1:YNCKHTQCWBTRNJIV4WNAE52SJUQCZO5C&xt.2=urn:sha1:TXGCZQTH26NL6OUQAJJPFALHG2LTGBC7
-         *     http://example.com:8800/zero/?6f09182b8ea51997#WtLEUO5Epj9UHAV9JFs+6pUQZp13TuspAUjnF+iM+dM=
-         *     http://user:example.com@localhost:8800/zero/?6f09182b8ea51997#WtLEUO5Epj9UHAV9JFs+6pUQZp13TuspAUjnF+iM+dM=
-         *
- * Attention: Does *not* sanitize HTML code! It is strongly advised to sanitize it after running this function. - * + * cache for script location * - * @name Helper.urls2links - * @function - * @param {String} html - HTML code + * @name Helper.baseUri + * @private + * @enum {string|null} */ - urls2links = function(html) - { - var markup = '$1'; - // short test: https://regex101.com/r/AttfVd/1 - html.replace( - /((http|https|ftp):\/\/[\w?=&.\/-;#@~%+*-]+(?![\w\s?&.\/;#~%"=-]*>))/ig, - markup - ) - // shorttest: https://regex101.com/r/sCm8Xe/2 - html.replace( - /((magnet):[\w?=&.\/-;#@~%+*-]+)/ig, - markup - ); - } + var baseUri = null; /** * converts a duration (in seconds) into human friendly approximation @@ -145,38 +135,55 @@ jQuery.PrivateBin = function($, sjcl, Base64, RawDeflate) { } /** - * set text of a jQuery element (required for IE) + * set text of a jQuery element (required for IE), * * @name Helper.setElementText * @function * @param {jQuery} $element - a jQuery element * @param {string} text - the text to enter - * @param {bool} convertLinks - whether to convert the links in the text */ - me.setElementText = function($element, text, convertLinks) + me.setElementText = function($element, text) { - var isIe = $('#oldienotice').is(':visible'); - // text-only and no IE -> fast way: set text-only - if ((convertLinks === false) && isIe === false) { - return $element.text(text); + // For IE<10: Doesn't support white-space:pre-wrap; so we have to do this... + if ($('#oldienotice').is(':visible')) { + var html = me.htmlEntities(text).replace(/\n/ig, '\r\n
'); + $element.html('
' + html + '
'); } - - // convert text to plain-text - // but as we need to handle HTML code afterwards - var html = $(text).text(); - - if (convertLinks === true) { - html = me.urls2links(html); - } - - // workaround: IE<10 doesn't support white-space:pre-wrap; so we have to do this... - if (isIe) { - html = html.replace(/\n/ig, '\r\n
'); + // for other (sane) browsers: + else + { + $element.text(text); } + } - // finally sanitize it for security (XSS) reasons - html = me.sanitizeHtml(text); - $element.html(html); + /** + * convert URLs to clickable links. + * URLs to handle: + * 
+         *     magnet:?xt.1=urn:sha1:YNCKHTQCWBTRNJIV4WNAE52SJUQCZO5C&xt.2=urn:sha1:TXGCZQTH26NL6OUQAJJPFALHG2LTGBC7
+         *     http://example.com:8800/zero/?6f09182b8ea51997#WtLEUO5Epj9UHAV9JFs+6pUQZp13TuspAUjnF+iM+dM=
+         *     http://user:example.com@localhost:8800/zero/?6f09182b8ea51997#WtLEUO5Epj9UHAV9JFs+6pUQZp13TuspAUjnF+iM+dM=
+         *
+ * + * @name Helper.urls2links + * @function + * @param {Object} $element - a jQuery DOM element + */ + me.urls2links = function($element) + { + var markup = '$1'; + $element.html( + $element.html().replace( + /((http|https|ftp):\/\/[\w?=&.\/-;#@~%+*-]+(?![\w\s?&.\/;#~%"=-]*>))/ig, + markup + ) + ); + $element.html( + $element.html().replace( + /((magnet):[\w?=&.\/-;#@~%+*-]+)/ig, + markup + ) + ); } /** @@ -263,17 +270,19 @@ jQuery.PrivateBin = function($, sjcl, Base64, RawDeflate) { } /** - * sanitizes html code to prevent XSS attacks + * convert all applicable characters to HTML entities * - * Now uses DOMPurify instead of some self-made stuff for security reasons. - * - * @name Helper.sanitizeHtml + * @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 Helper.htmlEntities * @function * @param {string} str * @return {string} escaped HTML */ - me.sanitizeHtml = function(str) { - return DOMPurify.sanitize(str, {SAFE_FOR_JQUERY: true}); + me.htmlEntities = function(str) { + return String(str).replace( + /[&<>"'`=\/]/g, function(s) { + return entityMap[s]; + }); } /** @@ -1757,8 +1766,9 @@ jQuery.PrivateBin = function($, sjcl, Base64, RawDeflate) { } // set text - Helper.setElementText($plainText, text, false); - Helper.setElementText($prettyPrint, text, true); + var sanitizedText = DOMPurify.sanitize(text, {SAFE_FOR_JQUERY: true}) + Helper.setElementText($plainText, sanitizedText); + Helper.setElementText($prettyPrint, sanitizedText); switch (format) { case 'markdown': @@ -1783,12 +1793,15 @@ jQuery.PrivateBin = function($, sjcl, Base64, RawDeflate) { $prettyPrint.html( prettyPrintOne( - Helper.sanitizeHtml(text), null, true + Helper.htmlEntities(sanitizedText), null, true ) ); // fall through, as the rest is the same default: // = 'plaintext' - // adjust CSS so it looks good + // convert URLs to clickable links + Helper.urls2links($plainText); + Helper.urls2links($prettyPrint); + $prettyPrint.css('white-space', 'pre-wrap'); $prettyPrint.css('word-break', 'normal'); $prettyPrint.removeClass('prettyprint'); @@ -2581,7 +2594,7 @@ jQuery.PrivateBin = function($, sjcl, Base64, RawDeflate) { for (var i = 0; i < $head.length; i++) { newDoc.write($head[i].outerHTML); } - newDoc.write('
' + Helper.sanitizeHtml(paste) + '
'); + newDoc.write('
' + Helper.htmlEntities(paste) + '
'); newDoc.close(); } diff --git a/js/test.js b/js/test.js index a48efdf..be4df6d 100644 --- a/js/test.js +++ b/js/test.js @@ -93,7 +93,7 @@ describe('Helper', function () { var html = '', result = true; ids.forEach(function(item, i) { - html += '
' + $.PrivateBin.Helper.sanitizeHtml(contents[i] || contents[0]) + '
'; + html += '
' + $.PrivateBin.Helper.htmlEntities(contents[i] || contents[0]) + '
'; }); var clean = jsdom(html); ids.forEach(function(item, i) { @@ -122,7 +122,7 @@ describe('Helper', function () { var html = '', result = true; ids.forEach(function(item, i) { - html += '
' + $.PrivateBin.Helper.sanitizeHtml(contents[i] || contents[0]) + '
'; + html += '
' + $.PrivateBin.Helper.htmlEntities(contents[i] || contents[0]) + '
'; }); var elements = $('').html(html); ids.forEach(function(item, i) { @@ -163,9 +163,9 @@ describe('Helper', function () { var query = query.join(''), fragment = fragment.join(''), url = schema + '://' + address.join('') + '/?' + query + '#' + fragment, - prefix = $.PrivateBin.Helper.sanitizeHtml(prefix), - postfix = ' ' + $.PrivateBin.Helper.sanitizeHtml(postfix), - element = '
' + prefix + url + postfix + '
'; + prefix = $.PrivateBin.Helper.htmlEntities(prefix), + postfix = ' ' + $.PrivateBin.Helper.htmlEntities(postfix), + element = $('
' + prefix + url + postfix + '
'); // special cases: When the query string and fragment imply the beginning of an HTML entity, eg. � or &#x if ( @@ -175,11 +175,11 @@ describe('Helper', function () { { url = schema + '://' + address.join('') + '/?' + query.substring(0, query.length - 1); postfix = ''; - element = '
' + prefix + url + '
'; + element = $('
' + prefix + url + '
'); } $.PrivateBin.Helper.urls2links(element); - return element.html() === '
' + prefix + '' + url + '' + postfix + '
'; + return element.html() === $('
' + prefix + '' + url + '' + postfix + '
').html(); } ); jsc.property( @@ -189,8 +189,8 @@ describe('Helper', function () { 'string', function (prefix, query, postfix) { var url = 'magnet:?' + query.join('').replace(/^&+|&+$/gm,''), - prefix = $.PrivateBin.Helper.sanitizeHtml(prefix), - postfix = $.PrivateBin.Helper.sanitizeHtml(postfix), + prefix = $.PrivateBin.Helper.htmlEntities(prefix), + postfix = $.PrivateBin.Helper.htmlEntities(postfix), element = $('
' + prefix + url + ' ' + postfix + '
'); $.PrivateBin.Helper.urls2links(element); return element.html() === $('
' + prefix + '' + url + ' ' + postfix + '
').html(); @@ -329,7 +329,7 @@ describe('Helper', function () { ); }); - describe('sanitizeHtml', function () { + describe('htmlEntities', function () { after(function () { cleanup(); }); @@ -338,7 +338,7 @@ describe('Helper', function () { 'removes all HTML entities from any given string', 'string', function (string) { - var result = $.PrivateBin.Helper.sanitizeHtml(string); + var result = $.PrivateBin.Helper.htmlEntities(string); return !(/[<>"'`=\/]/.test(result)) && !(string.indexOf('&') > -1 && !(/&/.test(result))); } ); @@ -583,8 +583,8 @@ describe('Model', function () { 'string', 'small nat', function (keys, value, key) { - keys = keys.map($.PrivateBin.Helper.sanitizeHtml); - value = $.PrivateBin.Helper.sanitizeHtml(value); + keys = keys.map($.PrivateBin.Helper.htmlEntities); + value = $.PrivateBin.Helper.htmlEntities(value); var content = keys.length > key ? keys[key] : (keys.length > 0 ? keys[0] : 'null'), contents = ''; $('body').html(contents); - var result = $.PrivateBin.Helper.sanitizeHtml( + var result = $.PrivateBin.Helper.htmlEntities( $.PrivateBin.Model.getExpirationDefault() ); $.PrivateBin.Model.reset(); @@ -617,8 +617,8 @@ describe('Model', function () { 'string', 'small nat', function (keys, value, key) { - keys = keys.map($.PrivateBin.Helper.sanitizeHtml); - value = $.PrivateBin.Helper.sanitizeHtml(value); + keys = keys.map($.PrivateBin.Helper.htmlEntities); + value = $.PrivateBin.Helper.htmlEntities(value); var content = keys.length > key ? keys[key] : (keys.length > 0 ? keys[0] : 'null'), contents = ''; $('body').html(contents); - var result = $.PrivateBin.Helper.sanitizeHtml( + var result = $.PrivateBin.Helper.htmlEntities( $.PrivateBin.Model.getFormatDefault() ); $.PrivateBin.Model.reset(); @@ -649,7 +649,7 @@ describe('Model', function () { 'checks if the element with id "cipherdata" contains any data', 'asciistring', function (value) { - value = $.PrivateBin.Helper.sanitizeHtml(value).trim(); + value = $.PrivateBin.Helper.htmlEntities(value).trim(); $('body').html('
' + value + '
'); $.PrivateBin.Model.init(); var result = $.PrivateBin.Model.hasCipherData(); @@ -669,10 +669,10 @@ describe('Model', function () { 'returns the contents of the element with id "cipherdata"', 'asciistring', function (value) { - value = $.PrivateBin.Helper.sanitizeHtml(value).trim(); + value = $.PrivateBin.Helper.htmlEntities(value).trim(); $('body').html('
' + value + '
'); $.PrivateBin.Model.init(); - var result = $.PrivateBin.Helper.sanitizeHtml( + var result = $.PrivateBin.Helper.htmlEntities( $.PrivateBin.Model.getCipherData() ); $.PrivateBin.Model.reset();