From 7e5a72703d061e41b0146137b80005cfd62ca73e Mon Sep 17 00:00:00 2001 From: Jamie Davis Date: Wed, 25 Apr 2018 20:03:48 -0400 Subject: [PATCH 1/3] security: rtrim, not unsafe /X+$/ idiom Problem: replace(/X+$/, '') is vulnerable to REDOS Solution: Replace all instances I could find with a custom rtrim --- lib/marked.js | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/lib/marked.js b/lib/marked.js index 83974865e4..50a6afa8a5 100644 --- a/lib/marked.js +++ b/lib/marked.js @@ -217,7 +217,7 @@ Lexer.prototype.token = function(src, top) { this.tokens.push({ type: 'code', text: !this.options.pedantic - ? cap.replace(/\n+$/, '') + ? rtrim(cap, '\n') : cap }); continue; @@ -1303,7 +1303,7 @@ function resolveUrl(base, href) { if (/^[^:]+:\/*[^/]*$/.test(base)) { baseUrls[' ' + base] = base + '/'; } else { - baseUrls[' ' + base] = base.replace(/[^/]*$/, ''); + baseUrls[' ' + base] = rtrim(base, '/', true); } } base = baseUrls[' ' + base]; @@ -1355,6 +1355,38 @@ function splitCells(tableRow, count) { return cells; } +// Return str with all trailing {c | all but c} removed +// allButC: Default false +function rtrim(str, c, allButC) { + if (typeof allButC === 'undefined') { + allButC = false; + } else { + allButC = true; + } + var mustMatchC = !allButC; + + if (str.length === 0) { + return ''; + } + + // ix+1 of leftmost that fits description + // i.e. the length of the string we should return + var curr = str.length; + + while (curr > 0) { + var currChar = str.charAt(curr - 1); + if (mustMatchC && currChar === c) { + curr--; + } else if (!mustMatchC && currChar !== c) { + curr--; + } else { + break; + } + } + + return str.substr(0, curr); +} + /** * Marked */ From 2e05c777ac5a4279500ba815ae0e8f8ace17eba5 Mon Sep 17 00:00:00 2001 From: Jamie Davis Date: Sat, 2 Jun 2018 01:17:38 -0400 Subject: [PATCH 2/3] address review comments --- lib/marked.js | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/lib/marked.js b/lib/marked.js index 50a6afa8a5..87c27a68bf 100644 --- a/lib/marked.js +++ b/lib/marked.js @@ -1355,36 +1355,36 @@ function splitCells(tableRow, count) { return cells; } -// Return str with all trailing {c | all but c} removed -// allButC: Default false -function rtrim(str, c, allButC) { - if (typeof allButC === 'undefined') { - allButC = false; +// Remove trailing 'c's. Equivalent to str.replace(/c*$/, ''). +// /c*$/ is vulnerable to REDOS. +// invert: Remove suffix of non-c chars instead. Default false. +function rtrim(str, c, invert) { + if (typeof invert === 'undefined' || !invert) { + invert = false; } else { - allButC = true; + invert = true; } - var mustMatchC = !allButC; if (str.length === 0) { return ''; } - // ix+1 of leftmost that fits description - // i.e. the length of the string we should return - var curr = str.length; + // Length of suffix matching the invert condition. + var suffLen = 0; - while (curr > 0) { - var currChar = str.charAt(curr - 1); - if (mustMatchC && currChar === c) { - curr--; - } else if (!mustMatchC && currChar !== c) { - curr--; + // Step left until we fail to match the invert condition. + while (suffLen < str.length) { + var currChar = str.charAt(str.length - suffLen - 1); + if (currChar === c && !invert) { + suffLen++; + } else if (currChar !== c && invert) { + suffLen++; } else { break; } } - return str.substr(0, curr); + return str.substr(0, str.length - suffLen); } /** From 0610f9f9a4b2b0050bd4f372a2ebdaee65adc352 Mon Sep 17 00:00:00 2001 From: Jamie Davis Date: Sat, 2 Jun 2018 14:12:17 -0400 Subject: [PATCH 3/3] remove unnecessary if check --- lib/marked.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/marked.js b/lib/marked.js index 87c27a68bf..837c586778 100644 --- a/lib/marked.js +++ b/lib/marked.js @@ -1357,14 +1357,8 @@ function splitCells(tableRow, count) { // Remove trailing 'c's. Equivalent to str.replace(/c*$/, ''). // /c*$/ is vulnerable to REDOS. -// invert: Remove suffix of non-c chars instead. Default false. +// invert: Remove suffix of non-c chars instead. Default falsey. function rtrim(str, c, invert) { - if (typeof invert === 'undefined' || !invert) { - invert = false; - } else { - invert = true; - } - if (str.length === 0) { return ''; }