-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Fix ReDOS #1405 #1408
Fix ReDOS #1405 #1408
Conversation
…y intended in 9f20c46). Reverts 9c976cd. Fixes markedjs#1405. Disable cm test 468.
If it helps at all: We're not using |
I don't think this regex for link is any more secure then the one in #1305. It just happens to fix this redos attack. I think this is a situation where we need to simplify the regex and use some other method to get the parts of the link, similar to what we do with tables. |
@UziTech sorry I totally forgot about this issue.
For every |
@Feder1co5oave yes that could be why it fixes this situation, but I was saying the regex in general is no more secure. In other words we will have to change the regex in a different way to make it secure in the future. A better solution would be to fix the regex (and possibly use some function instead of a single regex) now instead of in the future. |
How about a quick fix with this patch and then redo the regex into a function ? |
I'd like to hear from @davisjam on this |
We need this to be merged to fix the vulnerability snyk is advertising https://snyk.io/vuln/SNYK-JS-MARKED-73637 |
#1414 also fixes the vulnerability and still allows urls with matching parentheses |
Fixes #1405, reverts #1305
Something went wrong with the security review of that PR :(
I don't think we'll ever support multiple nested anything, anywhere. Regexes are just not made to do that easily.
I'd consider releasing this as a patch release to all minor versions available on npm.