Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

trim code after regexp #945

Closed
wants to merge 1 commit into from
Closed

trim code after regexp #945

wants to merge 1 commit into from

Conversation

UziTech
Copy link
Member

@UziTech UziTech commented Oct 10, 2017

Trimming the code after the regexp (instead of inside the regexp) seems to fix the regexp DOS described in #937

fixes #937

here is code to test it:

function genstr(len, chr) {
  var result = '';
  for (i=0; i<=len; i++) {
    result = result + chr;
  }
  return result;
}

var regex,start,output,end;

var input =  '`x' + genstr(50000, ' ') + 'x`';

regex = /^(`+)\s*([\s\S]*?[^`])\s*\1(?!`)/;
start = process.hrtime();
output = '<code>'+regex.exec(input)[2]+'</code>';
end = process.hrtime(start);

console.info(' DOS execution time (hr): %ds %dms', end[0], end[1] / 1000000);

regex = /^(`+)([\s\S]*?[^`])\1(?!`)/;
start = process.hrtime();
output = '<code>'+regex.exec(input)[2].trim()+'</code>';
end = process.hrtime(start);

console.info('Trim execution time (hr): %ds %dms', end[0], end[1] / 1000000);

@deksden deksden mentioned this pull request Oct 17, 2017
@kunagpal
Copy link

@UziTech Could you send this pull request to https://github.com/8fold/marked instead? It looks like this module is unmaintained 😞

@UziTech
Copy link
Member Author

UziTech commented Oct 17, 2017

Added the pull request https://github.com/8fold/marked/pull/1

@marcoscaceres
Copy link

@guypod or @maban, any chance of evaluating and adding this patch to @snyk? It might solve for https://snyk.io/vuln/npm:marked:20170907

@karenyavine
Copy link

Hey! We're already looking into it :)

@karenyavine
Copy link

We released a patch for this here: https://snyk.io/vuln/npm:marked:20170907
@marcoscaceres @UziTech

@marcoscaceres
Copy link

Amazing! Thanks so much @karenyavine! 🙏

@joshbruce joshbruce added this to Queue in PRs Dec 1, 2017
@UziTech
Copy link
Member Author

UziTech commented Jan 6, 2018

this was merged with v0.3.9

@UziTech UziTech closed this Jan 6, 2018
@joshbruce joshbruce moved this from Queue to Close in PRs Jan 22, 2018
@UziTech UziTech deleted the patch-1 branch April 5, 2018 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
PRs
Close
Development

Successfully merging this pull request may close these issues.

Vulnerable Regular Expression
4 participants