From 7f7260d021d978a2d103e0c430cc6dff576d0d04 Mon Sep 17 00:00:00 2001 From: Vitaly Puzrin Date: Thu, 13 Nov 2014 20:15:45 +0300 Subject: [PATCH] Better error handling in link normalizer + more tests for edge cases --- lib/helpers/normalize_link.js | 10 +++- lib/parser_inline.js | 10 ++-- .../fixtures/remarkable/commonmark_extras.txt | 9 ++++ test/fixtures/remarkable/xss.txt | 51 ++++++++++++++----- 4 files changed, 62 insertions(+), 18 deletions(-) diff --git a/lib/helpers/normalize_link.js b/lib/helpers/normalize_link.js index 557f7b9..2781f88 100644 --- a/lib/helpers/normalize_link.js +++ b/lib/helpers/normalize_link.js @@ -5,5 +5,13 @@ var replaceEntities = require('../common/utils').replaceEntities; module.exports = function normalizeLink(url) { - return encodeURI(decodeURI(replaceEntities(url))); + var normalized = replaceEntities(url); + + // We don't care much about result of mailformed URIs, + // but shoud not throw exception. + try { + normalized = decodeURI(normalized); + } catch (__) {} + + return encodeURI(normalized); }; diff --git a/lib/parser_inline.js b/lib/parser_inline.js index ad99182..755eafd 100644 --- a/lib/parser_inline.js +++ b/lib/parser_inline.js @@ -3,8 +3,9 @@ 'use strict'; -var Ruler = require('./ruler'); -var StateInline = require('./rules_inline/state_inline'); +var Ruler = require('./ruler'); +var StateInline = require('./rules_inline/state_inline'); +var replaceEntities = require('./common/utils').replaceEntities; //////////////////////////////////////////////////////////////////////////////// // Parser rules @@ -30,7 +31,10 @@ var _rules = [ var BAD_PROTOCOLS = [ 'vbscript', 'javascript', 'file' ]; function validateLink(url) { - var str = decodeURI(url).trim().toLowerCase(); + var str = url.trim().toLowerCase(); + + // Care about digital entities "javascript:alert(1)" + str = replaceEntities(str); if (str.indexOf(':') >= 0 && BAD_PROTOCOLS.indexOf(str.split(':')[0]) >= 0) { return false; diff --git a/test/fixtures/remarkable/commonmark_extras.txt b/test/fixtures/remarkable/commonmark_extras.txt index d9ed12e..d53c612 100644 --- a/test/fixtures/remarkable/commonmark_extras.txt +++ b/test/fixtures/remarkable/commonmark_extras.txt @@ -123,3 +123,12 @@ Autolinks do not allow escaping: .

http://example.com/\[\

. + + +Should not throw exception on mailformed URI + +. +[foo](<%test>) +. +

foo

+. \ No newline at end of file diff --git a/test/fixtures/remarkable/xss.txt b/test/fixtures/remarkable/xss.txt index db98624..2223d10 100644 --- a/test/fixtures/remarkable/xss.txt +++ b/test/fixtures/remarkable/xss.txt @@ -10,47 +10,70 @@ Should not allow some protocols in links and images . [xss link](javascript:alert(1)) + +[xss link](JAVASCRIPT:alert(1)) + +[xss link](vbscript:alert(1)) + +[xss link](VBSCRIPT:alert(1)) + +[xss link](file:///123) .

[xss link](javascript:alert(1))

+

[xss link](JAVASCRIPT:alert(1))

+

[xss link](vbscript:alert(1))

+

[xss link](VBSCRIPT:alert(1))

+

[xss link](file:///123)

. + . -[xss link](JAVASCRIPT:alert(1)) +[xss link]("><script>alert("xss")</script>) . -

[xss link](JAVASCRIPT:alert(1))

+

xss link

. . -[xss link](vbscript:alert(1)) +[xss link]() . -

[xss link](vbscript:alert(1))

+

[xss link](<javascript:alert(1)>)

. . -[xss link](VBSCRIPT:alert(1)) +[xss link](javascript:alert(1)) . -

[xss link](VBSCRIPT:alert(1))

+

[xss link](javascript:alert(1))

. + +Image parser use the same code base. + . -[xss link](file:///123) +![xss link](javascript:alert(1)) . -

[xss link](file:///123)

+

![xss link](javascript:alert(1))

. +Autolinks + . -[xss link]("><script>alert("xss")</script>) + + + . -

xss link

+

<javascript:alert(1)>

+

<javascript:alert(1)>

. -Image parser use the same code base. +Linkifier . -![xss link](javascript:alert(1)) +javascript:alert(1) + +javascript:alert(1) . -

![xss link](javascript:alert(1))

+

javascript:alert(1)

+

javascript:alert(1)

. -