From 4717be9e46efe9fc8e1b123d9cb4275a4c4e10b2 Mon Sep 17 00:00:00 2001 From: Jamie Davis Date: Sun, 15 Apr 2018 18:57:50 -0400 Subject: [PATCH 1/5] security: replace vulnerable regex with parser Problem: link regex was vulnerable Solution: dedicated parser Fixes: #1222 --- lib/marked.js | 45 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/lib/marked.js b/lib/marked.js index 8012063037..d1b126fe7f 100644 --- a/lib/marked.js +++ b/lib/marked.js @@ -554,9 +554,48 @@ inline.normal = merge({}, inline); inline.pedantic = merge({}, inline.normal, { strong: /^__(?=\S)([\s\S]*?\S)__(?!_)|^\*\*(?=\S)([\s\S]*?\S)\*\*(?!\*)/, em: /^_(?=\S)([\s\S]*?\S)_(?!_)|^\*(?=\S)([\s\S]*?\S)\*(?!\*)/, - link: edit(/^!?\[(label)\]\(\s*?(?:\s+(['"][\s\S]*?['"]))?\s*\)/) - .replace('label', inline._label) - .getRegex(), + link: { + exec: (s) => { + // [TEXT](DESTINATION) + var generalLinkRe = edit(/^!?\[(label)\]\((.*?)\)/) + .replace('label', inline._label) + .getRegex(); + + function unwrapCarats (str) { + if (str.match(/^<.*>$/)) { + str = str.substr(1, str.length - 1); + } + return str; + } + + var fullMatch = generalLinkRe.exec(s); + if (fullMatch) { + var text = fullMatch[1]; + var destination = fullMatch[2]; + + var m; + + var destinationAndTitleRe = /^([^'"(]*[^\s])\s+(['"(].*['")])/; + if (m = destinationAndTitleRe.exec(destination)) { + // -> destination + var dest1 = m[1].trim(); + dest1 = unwrapCarats(dest1); + var title1 = m[2]; + return [fullMatch[0], text, dest1, title1]; + } + + var destinationRe = /^(?)/; + if (m = destinationRe.exec(destination)) { + // -> destination + var dest2 = m[1].trim(); + destination = unwrapCarats(dest2); + var title2 = ''; + return [fullMatch[0], text, dest2, title2]; + } + } + return null; + } + }, reflink: edit(/^!?\[(label)\]\s*\[([^\]]*)\]/) .replace('label', inline._label) .getRegex() From 1ad9ca0fbea1cf0add0a1f4b22cd8f60fae2ac15 Mon Sep 17 00:00:00 2001 From: Jamie Davis Date: Sun, 15 Apr 2018 19:03:58 -0400 Subject: [PATCH 2/5] Travis failure: use backwards-compatible JS --- lib/marked.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/marked.js b/lib/marked.js index d1b126fe7f..9b1c27b751 100644 --- a/lib/marked.js +++ b/lib/marked.js @@ -555,7 +555,7 @@ inline.pedantic = merge({}, inline.normal, { strong: /^__(?=\S)([\s\S]*?\S)__(?!_)|^\*\*(?=\S)([\s\S]*?\S)\*\*(?!\*)/, em: /^_(?=\S)([\s\S]*?\S)_(?!_)|^\*(?=\S)([\s\S]*?\S)\*(?!\*)/, link: { - exec: (s) => { + exec: function (s) { // [TEXT](DESTINATION) var generalLinkRe = edit(/^!?\[(label)\]\((.*?)\)/) .replace('label', inline._label) From fbf93a82ff127bb9c13f012935eff894533111a6 Mon Sep 17 00:00:00 2001 From: Jamie Davis Date: Mon, 16 Apr 2018 10:02:19 -0400 Subject: [PATCH 3/5] address review comments --- lib/marked.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/marked.js b/lib/marked.js index 9b1c27b751..bb5afd018b 100644 --- a/lib/marked.js +++ b/lib/marked.js @@ -554,6 +554,9 @@ inline.normal = merge({}, inline); inline.pedantic = merge({}, inline.normal, { strong: /^__(?=\S)([\s\S]*?\S)__(?!_)|^\*\*(?=\S)([\s\S]*?\S)\*\*(?!\*)/, em: /^_(?=\S)([\s\S]*?\S)_(?!_)|^\*(?=\S)([\s\S]*?\S)\*(?!\*)/, + /* Original link re: /^!?\[(label)\]\(\s*?(?:\s+(['"][\s\S]*?['"]))?\s*\)/ + * This captures the spec reasonably well but is vulnerable to REDOS. + * Instead we use a custom parser that follows the RegExp.exec semantics. */ link: { exec: function (s) { // [TEXT](DESTINATION) @@ -561,9 +564,9 @@ inline.pedantic = merge({}, inline.normal, { .replace('label', inline._label) .getRegex(); - function unwrapCarats (str) { + function unwrapAngleBrackets (str) { if (str.match(/^<.*>$/)) { - str = str.substr(1, str.length - 1); + str = str.slice(1, -1); } return str; } @@ -579,7 +582,7 @@ inline.pedantic = merge({}, inline.normal, { if (m = destinationAndTitleRe.exec(destination)) { // -> destination var dest1 = m[1].trim(); - dest1 = unwrapCarats(dest1); + dest1 = unwrapAngleBrackets(dest1); var title1 = m[2]; return [fullMatch[0], text, dest1, title1]; } @@ -588,7 +591,7 @@ inline.pedantic = merge({}, inline.normal, { if (m = destinationRe.exec(destination)) { // -> destination var dest2 = m[1].trim(); - destination = unwrapCarats(dest2); + dest2 = unwrapAngleBrackets(dest2); var title2 = ''; return [fullMatch[0], text, dest2, title2]; } From ba2fc132f293899141fb65b2ed99dabf3e8107d6 Mon Sep 17 00:00:00 2001 From: Jamie Davis Date: Mon, 16 Apr 2018 10:33:01 -0400 Subject: [PATCH 4/5] address review comments --- lib/marked.js | 65 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 22 deletions(-) diff --git a/lib/marked.js b/lib/marked.js index bb5afd018b..4e1934359a 100644 --- a/lib/marked.js +++ b/lib/marked.js @@ -564,11 +564,46 @@ inline.pedantic = merge({}, inline.normal, { .replace('label', inline._label) .getRegex(); - function unwrapAngleBrackets (str) { - if (str.match(/^<.*>$/)) { - str = str.slice(1, -1); + // destination: DESTINATION from generalLinkRe + // returns [destination, title]: no angle-brackets on destination, no quotes on title + function splitIntoDestinationAndTitle (destination) { + function unwrapAngleBrackets (str) { + if (str.match(/^<.*>$/)) { + str = str.slice(1, -1); + } + return str; + } + + // Valid DESTINATIONs, in decreasing specificity. + var destinationAndTitleRe = /^([^'"(]*[^\s])\s+(['"(].*['")])/; + var destinationRe = /^(?)/; + var parsingRegexes = [destinationAndTitleRe, destinationRe]; + + var match = false; + var dest = undefined; + var title = undefined; + for (var i = 0; i < parsingRegexes.length; i++) { + match = parsingRegexes[i].exec(destination); + if (match) { + dest = match[1]; + title = match[2]; + break; + } + } + + if (match) { + // title is optional. + if (typeof title === 'undefined') { + title = ''; + } + + // Format dest. + dest = dest.trim(); + dest = unwrapAngleBrackets(dest); + + return [dest, title]; } - return str; + return null; } var fullMatch = generalLinkRe.exec(s); @@ -576,24 +611,10 @@ inline.pedantic = merge({}, inline.normal, { var text = fullMatch[1]; var destination = fullMatch[2]; - var m; - - var destinationAndTitleRe = /^([^'"(]*[^\s])\s+(['"(].*['")])/; - if (m = destinationAndTitleRe.exec(destination)) { - // -> destination - var dest1 = m[1].trim(); - dest1 = unwrapAngleBrackets(dest1); - var title1 = m[2]; - return [fullMatch[0], text, dest1, title1]; - } - - var destinationRe = /^(?)/; - if (m = destinationRe.exec(destination)) { - // -> destination - var dest2 = m[1].trim(); - dest2 = unwrapAngleBrackets(dest2); - var title2 = ''; - return [fullMatch[0], text, dest2, title2]; + // Does 'destination' match spec? + var destinationAndTitle = splitIntoDestinationAndTitle(destination); + if (destinationAndTitle) { + return [fullMatch[0], text, destinationAndTitle[0], destinationAndTitle[1]]; } } return null; From 47f4388cc112af03a60ec4101a3e98d7f73a73db Mon Sep 17 00:00:00 2001 From: Jamie Davis Date: Mon, 16 Apr 2018 11:09:33 -0400 Subject: [PATCH 5/5] address review comments --- lib/marked.js | 46 +++++++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/lib/marked.js b/lib/marked.js index 4e1934359a..74c45c7d9f 100644 --- a/lib/marked.js +++ b/lib/marked.js @@ -580,44 +580,40 @@ inline.pedantic = merge({}, inline.normal, { var parsingRegexes = [destinationAndTitleRe, destinationRe]; var match = false; - var dest = undefined; - var title = undefined; for (var i = 0; i < parsingRegexes.length; i++) { match = parsingRegexes[i].exec(destination); if (match) { - dest = match[1]; - title = match[2]; break; } } - if (match) { - // title is optional. - if (typeof title === 'undefined') { - title = ''; - } + if (!match) { + return null; + } - // Format dest. - dest = dest.trim(); - dest = unwrapAngleBrackets(dest); + var dest = match[1]; + var title = match[2] || ''; // Not all parsingRegexes have 2 groups. - return [dest, title]; - } - return null; + // Format dest. + dest = dest.trim(); + dest = unwrapAngleBrackets(dest); + + return [dest, title]; } var fullMatch = generalLinkRe.exec(s); - if (fullMatch) { - var text = fullMatch[1]; - var destination = fullMatch[2]; - - // Does 'destination' match spec? - var destinationAndTitle = splitIntoDestinationAndTitle(destination); - if (destinationAndTitle) { - return [fullMatch[0], text, destinationAndTitle[0], destinationAndTitle[1]]; - } + if (!fullMatch) { + return null; + } + + var text = fullMatch[1]; + var destination = fullMatch[2]; + + var destinationAndTitle = splitIntoDestinationAndTitle(destination); + if (!destinationAndTitle) { + return null; } - return null; + return [fullMatch[0], text, destinationAndTitle[0], destinationAndTitle[1]]; } }, reflink: edit(/^!?\[(label)\]\s*\[([^\]]*)\]/)