From 8d2e19f7917231dee1af2453f55ddbd21fd83af5 Mon Sep 17 00:00:00 2001 From: rugk Date: Wed, 22 Nov 2017 16:48:00 +0100 Subject: [PATCH] Try to move sanitisation & links into setElementText --- js/privatebin.js | 141 +++++++++++++++++++++-------------------------- js/test.js | 40 +++++++------- 2 files changed, 84 insertions(+), 97 deletions(-) diff --git a/js/privatebin.js b/js/privatebin.js index 86b6046..df5dffc 100644 --- a/js/privatebin.js +++ b/js/privatebin.js @@ -43,26 +43,6 @@ jQuery.PrivateBin = function($, sjcl, Base64, RawDeflate) { var Helper = (function () { var me = {}; - /** - * 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 = { - '&': '&', - '<': '<', - '>': '>', - '"': '"', - "'": ''', - '/': '/', - '`': '`', - '=': '=' - }; - /** * cache for script location * @@ -72,6 +52,36 @@ jQuery.PrivateBin = function($, sjcl, Base64, RawDeflate) { */ var baseUri = null; + /** + * 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. + * + * + * @name Helper.urls2links + * @function + * @param {String} html - HTML code + */ + 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 + ); + } + /** * converts a duration (in seconds) into human friendly approximation * @@ -135,55 +145,38 @@ 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) + me.setElementText = function($element, text, convertLinks) { - // 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 + '
'); + 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 other (sane) browsers: - else - { - $element.text(text); - } - } - /** - * 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 - ) - ); + // 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
'); + } + + // finally sanitize it for security (XSS) reasons + html = me.sanitizeHtml(text); + $element.html(html); } /** @@ -270,19 +263,17 @@ jQuery.PrivateBin = function($, sjcl, Base64, RawDeflate) { } /** - * convert all applicable characters to HTML entities + * sanitizes html code to prevent XSS attacks * - * @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 + * Now uses DOMPurify instead of some self-made stuff for security reasons. + * + * @name Helper.sanitizeHtml * @function * @param {string} str * @return {string} escaped HTML */ - me.htmlEntities = function(str) { - return String(str).replace( - /[&<>"'`=\/]/g, function(s) { - return entityMap[s]; - }); + me.sanitizeHtml = function(str) { + return DOMPurify.sanitize(str, {SAFE_FOR_JQUERY: true}); } /** @@ -1766,9 +1757,8 @@ jQuery.PrivateBin = function($, sjcl, Base64, RawDeflate) { } // set text - var sanitizedText = DOMPurify.sanitize(text, {SAFE_FOR_JQUERY: true}) - Helper.setElementText($plainText, sanitizedText); - Helper.setElementText($prettyPrint, sanitizedText); + Helper.setElementText($plainText, text, false); + Helper.setElementText($prettyPrint, text, true); switch (format) { case 'markdown': @@ -1793,15 +1783,12 @@ jQuery.PrivateBin = function($, sjcl, Base64, RawDeflate) { $prettyPrint.html( prettyPrintOne( - Helper.htmlEntities(sanitizedText), null, true + Helper.sanitizeHtml(text), null, true ) ); // fall through, as the rest is the same default: // = 'plaintext' - // convert URLs to clickable links - Helper.urls2links($plainText); - Helper.urls2links($prettyPrint); - + // adjust CSS so it looks good $prettyPrint.css('white-space', 'pre-wrap'); $prettyPrint.css('word-break', 'normal'); $prettyPrint.removeClass('prettyprint'); @@ -2594,7 +2581,7 @@ jQuery.PrivateBin = function($, sjcl, Base64, RawDeflate) { for (var i = 0; i < $head.length; i++) { newDoc.write($head[i].outerHTML); } - newDoc.write('
' + Helper.htmlEntities(paste) + '
'); + newDoc.write('
' + Helper.sanitizeHtml(paste) + '
'); newDoc.close(); } diff --git a/js/test.js b/js/test.js index be4df6d..a48efdf 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.htmlEntities(contents[i] || contents[0]) + '
'; + html += '
' + $.PrivateBin.Helper.sanitizeHtml(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.htmlEntities(contents[i] || contents[0]) + '
'; + html += '
' + $.PrivateBin.Helper.sanitizeHtml(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.htmlEntities(prefix), - postfix = ' ' + $.PrivateBin.Helper.htmlEntities(postfix), - element = $('
' + prefix + url + postfix + '
'); + prefix = $.PrivateBin.Helper.sanitizeHtml(prefix), + postfix = ' ' + $.PrivateBin.Helper.sanitizeHtml(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 + '
').html(); + return element.html() === '
' + prefix + '' + url + '' + postfix + '
'; } ); jsc.property( @@ -189,8 +189,8 @@ describe('Helper', function () { 'string', function (prefix, query, postfix) { var url = 'magnet:?' + query.join('').replace(/^&+|&+$/gm,''), - prefix = $.PrivateBin.Helper.htmlEntities(prefix), - postfix = $.PrivateBin.Helper.htmlEntities(postfix), + prefix = $.PrivateBin.Helper.sanitizeHtml(prefix), + postfix = $.PrivateBin.Helper.sanitizeHtml(postfix), element = $('
' + prefix + url + ' ' + postfix + '
'); $.PrivateBin.Helper.urls2links(element); return element.html() === $('
' + prefix + '' + url + ' ' + postfix + '
').html(); @@ -329,7 +329,7 @@ describe('Helper', function () { ); }); - describe('htmlEntities', function () { + describe('sanitizeHtml', 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.htmlEntities(string); + var result = $.PrivateBin.Helper.sanitizeHtml(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.htmlEntities); - value = $.PrivateBin.Helper.htmlEntities(value); + keys = keys.map($.PrivateBin.Helper.sanitizeHtml); + value = $.PrivateBin.Helper.sanitizeHtml(value); var content = keys.length > key ? keys[key] : (keys.length > 0 ? keys[0] : 'null'), contents = ''; $('body').html(contents); - var result = $.PrivateBin.Helper.htmlEntities( + var result = $.PrivateBin.Helper.sanitizeHtml( $.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.htmlEntities); - value = $.PrivateBin.Helper.htmlEntities(value); + keys = keys.map($.PrivateBin.Helper.sanitizeHtml); + value = $.PrivateBin.Helper.sanitizeHtml(value); var content = keys.length > key ? keys[key] : (keys.length > 0 ? keys[0] : 'null'), contents = ''; $('body').html(contents); - var result = $.PrivateBin.Helper.htmlEntities( + var result = $.PrivateBin.Helper.sanitizeHtml( $.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.htmlEntities(value).trim(); + value = $.PrivateBin.Helper.sanitizeHtml(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.htmlEntities(value).trim(); + value = $.PrivateBin.Helper.sanitizeHtml(value).trim(); $('body').html('
' + value + '
'); $.PrivateBin.Model.init(); - var result = $.PrivateBin.Helper.htmlEntities( + var result = $.PrivateBin.Helper.sanitizeHtml( $.PrivateBin.Model.getCipherData() ); $.PrivateBin.Model.reset();