From cfad82ec3e7c29e64d50815932c19ae6e0388615 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorenz=20H=C3=BCbschle-Schneider?= Date: Mon, 8 Jun 2015 23:35:18 +0200 Subject: [PATCH 1/2] Properly fix #622 by escaping input into DOM filters --- index.html | 4 ++-- js/filters.js | 36 +++++++++++++++--------------------- 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/index.html b/index.html index 0a71a8f..0db107f 100644 --- a/index.html +++ b/index.html @@ -21,7 +21,7 @@ - + @@ -284,7 +284,7 @@ $ openssl req -nodes -newkey rsa:4096 -keyout relay.pem -x509 -days 365 -out rel <>
+ --> diff --git a/js/filters.js b/js/filters.js index d2b0d96..0b7abc5 100644 --- a/js/filters.js +++ b/js/filters.js @@ -30,13 +30,6 @@ weechat.filter('irclinky', ['$filter', function($filter) { return text; } - // First, escape entities to prevent escaping issues because it's a bad idea - // to parse/modify HTML with regexes, which we do a couple of lines down... - var entities = {"<": "<", ">": ">", '"': '"', "'": ''', "&": "&", "/": '/'}; - text = text.replace(/[<>"'&\/]/g, function (char) { - return entities[char]; - }); - // This regex in no way matches all IRC channel names (they could also begin with &, + or an // exclamation mark followed by 5 alphanumeric characters, and are bounded in length by 50). // However, it matches all *common* IRC channels while trying to minimise false positives. @@ -73,6 +66,15 @@ weechat.filter('DOMfilter', ['$filter', '$sce', function($filter, $sce) { return text; } + var escape_html = function(text) { + // First, escape entities to prevent escaping issues because it's a bad idea + // to parse/modify HTML with regexes, which we do a couple of lines down... + var entities = {"<": "<", ">": ">", '"': '"', "'": ''', "&": "&", "/": '/'}; + return text.replace(/[<>"'&\/]/g, function (char) { + return entities[char]; + }); + }; + // hacky way to pass extra arguments without using .apply, which // would require assembling an argument array. PERFORMANCE!!! var extraArgument = (arguments.length > 2) ? arguments[2] : null; @@ -85,8 +87,12 @@ weechat.filter('DOMfilter', ['$filter', '$sce', function($filter, $sce) { // Recursive DOM-walking function applying the filter to the text nodes var process = function(node) { if (node.nodeType === 3) { // text node - var value = filterFunction(node.nodeValue, extraArgument, thirdArgument); - if (value !== node.nodeValue) { + // apply the filter to *escaped* HTML, and only commit changes if + // it changed the escaped value. This is because setting the result + // as innerHTML causes it to be unescaped. + var input = escape_html(node.nodeValue); + var value = filterFunction(input, extraArgument, thirdArgument); + if (value !== input) { // we changed something. create a new node to replace the current one // we could also only add its children but that would probably incur // more overhead than it would gain us @@ -141,18 +147,6 @@ weechat.filter('getBufferQuickKeys', function () { }; }); -weechat.filter('escape', ['$sanitize', function($sanitize) { - return function(text) { - // manual escaping because ng-sanitize is shit - return text - .replace(/&/g, "&") - .replace(//g, ">") - .replace(/"/g, """) - .replace(/'/g, "'"); - }; -}]); - // Emojifis the string using https://github.com/Ranks/emojione weechat.filter('emojify', function() { return function(text, enable_JS_Emoji) { From 420836974e6c782096d4363f524abefe204d468f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorenz=20H=C3=BCbschle-Schneider?= Date: Mon, 8 Jun 2015 23:42:57 +0200 Subject: [PATCH 2/2] Fix irclinky test irclinky no longer does escaping itself, that is now done by DOMfilter. Adapt the test case to reflect this change. --- test/unit/filters.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/filters.js b/test/unit/filters.js index 934996e..c8ea29e 100644 --- a/test/unit/filters.js +++ b/test/unit/filters.js @@ -20,7 +20,7 @@ describe('Filters', function() { })); it('should not mess up IRC channels surrounded by HTML entities', inject(function(irclinkyFilter) { - expect(irclinkyFilter('<"#foo">')).toEqual('<"#foo">'); + expect(irclinkyFilter('<"#foo">')).toEqual('<"\'); $scope.$apply();">#foo">'); })); });