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

ReDoS on link regex #29

Closed
night opened this issue Jun 26, 2017 · 5 comments
Closed

ReDoS on link regex #29

night opened this issue Jun 26, 2017 · 5 comments

Comments

@night
Copy link

night commented Jun 26, 2017

The regex for the link rule is subject to DoS when there is a malformed link.

Working repro:

SimpleMarkdown.defaultBlockParse('[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n');
@ariabuckles
Copy link
Owner

Sorry for the delay; I've spent the past several months trying to bring a startup to launch.

Yikes! This looks like a real bug, thanks for pointing it out and I'll look into it.

@ariabuckles
Copy link
Owner

Figured out the issue; I'll have a fix for this out by monday. Sorry for the month lag D:

ariabuckles added a commit that referenced this issue Sep 3, 2017
Makes the same modification from marked in this commit:
markedjs/marked@d53f206

Fixes issue #29

Test plan:
Run the following in node, and get output instantly:

```
require('./simple-markdown.js').defaultHtmlOutput(require('./simple-markdown.js').defaultBlockParse('[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n'));
```
ariabuckles added a commit that referenced this issue Sep 3, 2017
Makes the same modification from marked in this commit:
markedjs/marked@d53f206

Fixes issue #29

Test plan:
Run the following in node, and get output instantly:

```
require('./simple-markdown.js').defaultHtmlOutput(require('./simple-markdown.js').defaultBlockParse('[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n[+test)[]\n'));
```
@ariabuckles
Copy link
Owner

Hi again!

This is fixed in version 0.2.2 (on npm), which is now out on npm.

I also reworked my bold/italic nested parsing solution so that it's backwards compatible, and released that as 0.3.0 (which includes 0.2.2/the fix for this issue). This lets simple-markdown correctly parse ***nested* bold and italics**. This should just work (no breaking changes), but it is a lot of code changed. So if you'd be interested in testing that bold/italic/your custom rules work and letting me know if you run any problems, I'd love to fix them.

If this isn't a good time to test a more significant change, just updating to 0.2.2 fixes just this issue.

Thanks for your work on discord & for letting me know about simple-markdown issues~!

@night
Copy link
Author

night commented Sep 3, 2017

@ariabuckles thanks for your continued support of this module! we definitely exploit simple-markdown to the best of its ability and it's quite great for what it does. this week i will bump the version and throw it onto canary if things look alright locally (canary is our cutting edge public build that people are eager to try and report bugs for). i'll be sure to report any issues encountered 🤞

@ariabuckles
Copy link
Owner

Awesome, thanks @night! I'm happy it's been useful to you :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants