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

Markdown parsing vulnerable to ReDoS attack #21

Open
teaishealthy opened this issue Oct 11, 2023 · 0 comments
Open

Markdown parsing vulnerable to ReDoS attack #21

teaishealthy opened this issue Oct 11, 2023 · 0 comments

Comments

@teaishealthy
Copy link

Markdown parsing vulnerable to ReDoS attack

Description

There is a currently minor ReDoS vulnerability against markdown parsing caused by this specific pattern

html = html.replace(/(\S+)```(\S+ ?)?/gm, (_, p1, p2, offset) => {

Inspect it on recheck.

Attack example

const string = '``a'.repeat(amount) + '``\t````';  
const pattern = /(\S+)```(\S+ ?)?/gm;

console.time();
string.match(pattern)
console.timeEnd();

amount = 664 for the current max message length but with an max message length of e.g. 5000, ~10 messages per second are enough to completely block the event loop. This is made even worse by the fact that all messages are sent at once after a rate-limit expires, so the actual blocking time is much higher even for a small message cap.

Rate-limits help mitigate the issue, but currently one account sending malicious messages is enough to get the client to lag/stutter, as the browser does not have enough time to paint for the client appear smoothly. This will only get worse with higher character limits (e.g. premium) or more malicious accounts, and more features in the client.

Fixing the issue

Find a regex that isn't vulnerable to backtracking like this one.

How can this be avoided in the future

Check your regexes with recheck and/use the recheck eslint plugin.

Notes

All other regexes seem to be fine.

The regexes in the backend are handled using a automata, meaning linear time complexity instead of polynomial/exponential. I checked them anyway, all good 🚀

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

1 participant