From a0740ff79f9076ec7fa4d80bdfb32337a7136482 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Wed, 22 Nov 2017 22:27:38 +0100 Subject: [PATCH] getting rid of htmlEntities (except for tests) and setElementText (dropping IE9 support), changing urls2links interface, all to avoid double encoding sanitized HTML --- js/privatebin.js | 107 +++++++++------------------------------------- js/test.js | 104 ++++++++++++++++++++++---------------------- tpl/bootstrap.php | 2 +- tpl/page.php | 2 +- 4 files changed, 73 insertions(+), 142 deletions(-) diff --git a/js/privatebin.js b/js/privatebin.js index 86b6046e..7b141c51 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 * @@ -134,28 +114,6 @@ jQuery.PrivateBin = function($, sjcl, Base64, RawDeflate) { } } - /** - * 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 - */ - me.setElementText = function($element, 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 + '
'); - } - // for other (sane) browsers: - else - { - $element.text(text); - } - } - /** * convert URLs to clickable links. * URLs to handle: @@ -167,22 +125,14 @@ jQuery.PrivateBin = function($, sjcl, Base64, RawDeflate) { * * @name Helper.urls2links * @function - * @param {Object} $element - a jQuery DOM element + * @param {string} html + * @return {string} */ - me.urls2links = function($element) + me.urls2links = function(html) { - 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 - ) + return html.replace( + /(((http|https|ftp):\/\/[\w?=&.\/-;#@~%+*-]+(?![\w\s?&.\/;#~%"=-]*>))|((magnet):[\w?=&.\/-;#@~%+*-]+))/ig, + '$1' ); } @@ -269,22 +219,6 @@ jQuery.PrivateBin = function($, sjcl, Base64, RawDeflate) { return baseUri; } - /** - * 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 Helper.htmlEntities - * @function - * @param {string} str - * @return {string} escaped HTML - */ - me.htmlEntities = function(str) { - return String(str).replace( - /[&<>"'`=\/]/g, function(s) { - return entityMap[s]; - }); - } - /** * resets state, used for unit testing * @@ -1765,10 +1699,10 @@ jQuery.PrivateBin = function($, sjcl, Base64, RawDeflate) { return; } - // set text - var sanitizedText = DOMPurify.sanitize(text, {SAFE_FOR_JQUERY: true}) - Helper.setElementText($plainText, sanitizedText); - Helper.setElementText($prettyPrint, sanitizedText); + // set sanitized and linked text + var sanitizedLinkedText = DOMPurify.sanitize(Helper.urls2links(text), {SAFE_FOR_JQUERY: true}); + $plainText.html(sanitizedLinkedText); + $prettyPrint.html(sanitizedLinkedText); switch (format) { case 'markdown': @@ -1785,23 +1719,20 @@ jQuery.PrivateBin = function($, sjcl, Base64, RawDeflate) { $plainText.find('table').addClass('table-condensed table-bordered'); break; case 'syntaxhighlighting': - // @TODO is this really needed or is "one" enough? + // yes, this is really needed to initialize the environment if (typeof prettyPrint === 'function') { prettyPrint(); } $prettyPrint.html( - prettyPrintOne( - Helper.htmlEntities(sanitizedText), null, true + DOMPurify.sanitize( + prettyPrintOne(Helper.urls2links(text), null, true), + {SAFE_FOR_JQUERY: true} ) ); // fall through, as the rest is the same default: // = 'plaintext' - // 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'); @@ -2290,8 +2221,12 @@ jQuery.PrivateBin = function($, sjcl, Base64, RawDeflate) { var $commentEntryData = $commentEntry.find('div.commentdata'); // set & parse text - Helper.setElementText($commentEntryData, commentText); - Helper.urls2links($commentEntryData); + $commentEntryData.html( + DOMPurify.sanitize( + Helper.urls2links(commentText), + {SAFE_FOR_JQUERY: true} + ) + ); // set nickname if (nickname.length > 0) { @@ -2594,7 +2529,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('
' + DOMPurify.sanitize(paste, {SAFE_FOR_JQUERY: true}) + '
'); newDoc.close(); } diff --git a/js/test.js b/js/test.js index 99180fac..06535f05 100644 --- a/js/test.js +++ b/js/test.js @@ -15,6 +15,22 @@ var jsc = require('jsverify'), // schemas supported by the whatwg-url library schemas = ['ftp','gopher','http','https','ws','wss'], supportedLanguages = ['de', 'es', 'fr', 'it', 'no', 'pl', 'pt', 'oc', 'ru', 'sl', 'zh'], + + /** + * character to HTML entity lookup table + * + * @see {@link https://github.com/janl/mustache.js/blob/master/mustache.js#L60} + */ + entityMap = { + '&': '&', + '<': '<', + '>': '>', + '"': '"', + "'": ''', + '/': '/', + '`': '`', + '=': '=' + }, logFile = require('fs').createWriteStream('test.log'); global.$ = global.jQuery = require('./jquery-3.1.1'); @@ -35,6 +51,22 @@ console.info = console.warn = console.error = function () { logFile.write(Array.prototype.slice.call(arguments).join('') + '\n'); } +/** + * 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 + */ +function htmlEntities(str) { + return String(str).replace( + /[&<>"'`=\/]/g, function(s) { + return entityMap[s]; + }); +} + describe('Helper', function () { describe('secondsToHuman', function () { after(function () { @@ -93,7 +125,7 @@ describe('Helper', function () { var html = '', result = true; ids.forEach(function(item, i) { - html += '
' + $.PrivateBin.Helper.htmlEntities(contents[i] || contents[0]) + '
'; + html += '
' + htmlEntities(contents[i] || contents[0]) + '
'; }); var clean = jsdom(html); ids.forEach(function(item, i) { @@ -108,34 +140,6 @@ describe('Helper', function () { ); }); - describe('setElementText', function () { - after(function () { - cleanup(); - }); - - jsc.property( - 'replaces the content of an element', - jsc.nearray(jsc.nearray(jsc.elements(alnumString))), - 'nearray string', - 'string', - function (ids, contents, replacingContent) { - var html = '', - result = true; - ids.forEach(function(item, i) { - html += '
' + $.PrivateBin.Helper.htmlEntities(contents[i] || contents[0]) + '
'; - }); - var elements = $('').html(html); - ids.forEach(function(item, i) { - var id = item.join(''), - element = elements.find('#' + id).first(); - $.PrivateBin.Helper.setElementText(element, replacingContent); - result *= replacingContent === element.text(); - }); - return Boolean(result); - } - ); - }); - describe('urls2links', function () { after(function () { cleanup(); @@ -145,10 +149,7 @@ describe('Helper', function () { 'ignores non-URL content', 'string', function (content) { - var element = $('
' + content + '
'), - before = element.html(); - $.PrivateBin.Helper.urls2links(element); - return before === element.html(); + return content === $.PrivateBin.Helper.urls2links(content); } ); jsc.property( @@ -163,9 +164,8 @@ 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 = htmlEntities(prefix), + postfix = ' ' + htmlEntities(postfix); // special cases: When the query string and fragment imply the beginning of an HTML entity, eg. � or &#x if ( @@ -175,11 +175,9 @@ describe('Helper', function () { { url = schema + '://' + address.join('') + '/?' + query.substring(0, query.length - 1); postfix = ''; - element = $('
' + prefix + url + '
'); } - $.PrivateBin.Helper.urls2links(element); - return element.html() === $('
' + prefix + '' + url + '' + postfix + '
').html(); + return prefix + '' + url + '' + postfix === $.PrivateBin.Helper.urls2links(prefix + url + postfix); } ); jsc.property( @@ -189,11 +187,9 @@ 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), - element = $('
' + prefix + url + ' ' + postfix + '
'); - $.PrivateBin.Helper.urls2links(element); - return element.html() === $('
' + prefix + '' + url + ' ' + postfix + '
').html(); + prefix = htmlEntities(prefix), + postfix = htmlEntities(postfix); + return prefix + '' + url + ' ' + postfix === $.PrivateBin.Helper.urls2links(prefix + url + ' ' + postfix); } ); }); @@ -338,7 +334,7 @@ describe('Helper', function () { 'removes all HTML entities from any given string', 'string', function (string) { - var result = $.PrivateBin.Helper.htmlEntities(string); + var result = htmlEntities(string); return !(/[<>"'`=\/]/.test(result)) && !(string.indexOf('&') > -1 && !(/&/.test(result))); } ); @@ -583,8 +579,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(htmlEntities); + value = htmlEntities(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 = htmlEntities( $.PrivateBin.Model.getExpirationDefault() ); $.PrivateBin.Model.reset(); @@ -617,8 +613,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(htmlEntities); + value = htmlEntities(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 = htmlEntities( $.PrivateBin.Model.getFormatDefault() ); $.PrivateBin.Model.reset(); @@ -649,7 +645,7 @@ describe('Model', function () { 'checks if the element with id "cipherdata" contains any data', 'asciistring', function (value) { - value = $.PrivateBin.Helper.htmlEntities(value).trim(); + value = htmlEntities(value).trim(); $('body').html('
' + value + '
'); $.PrivateBin.Model.init(); var result = $.PrivateBin.Model.hasCipherData(); @@ -669,10 +665,10 @@ describe('Model', function () { 'returns the contents of the element with id "cipherdata"', 'asciistring', function (value) { - value = $.PrivateBin.Helper.htmlEntities(value).trim(); + value = htmlEntities(value).trim(); $('body').html('
' + value + '
'); $.PrivateBin.Model.init(); - var result = $.PrivateBin.Helper.htmlEntities( + var result = htmlEntities( $.PrivateBin.Model.getCipherData() ); $.PrivateBin.Model.reset(); diff --git a/tpl/bootstrap.php b/tpl/bootstrap.php index 89cd4ff6..203fbcba 100644 --- a/tpl/bootstrap.php +++ b/tpl/bootstrap.php @@ -70,7 +70,7 @@ if ($MARKDOWN): - + diff --git a/tpl/page.php b/tpl/page.php index c59fba71..867f8ed0 100644 --- a/tpl/page.php +++ b/tpl/page.php @@ -48,7 +48,7 @@ if ($MARKDOWN): - +