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

Mermaid hangs up current page on large inputs #1216

Closed
rajat-gl opened this issue Jan 23, 2020 · 3 comments · Fixed by #1221
Closed

Mermaid hangs up current page on large inputs #1216

rajat-gl opened this issue Jan 23, 2020 · 3 comments · Fixed by #1221
Assignees
Labels
Status: Approved Is ready to be worked on Type: Bug / Error Something isn't working or is incorrect

Comments

@rajat-gl
Copy link

Overview
When we send in large input to Mermaid for rendering, mermaid hangs up the entire page which results in page being unresponsive.

Implications can be huge when a website saves the mermaid code and displays it to different users. This can cause a denial of service attack.

Problematic scenario

  • A user enters an unusually large mermaid code:
    mermaid.txt
  • We upload and save the code in our database.
  • When another user visits the webpage, we load the unusually large input and try to render it via Mermaid.js. Mermaid bails out and causes the page to hang up.
  • This repeats for all the users who have access to the page where we try to load mermaid js.

To Reproduce
Steps to reproduce the behavior:

  1. Go to https://mermaid-js.github.io/mermaid-live-editor
  2. Download the above Mermaid.txt file. Copy the text and paste it inside the Load sample diagram textarea.
  3. Wait a few moments, hoping for the graph to show up.
  4. See this instead

Page unresponsive

Expected behavior
Mermaid render should not hang up the page, it should either stop rendering when it determines that the input is too large or don't start the render at all.

Screenshots
See "To Reproduce" section.

Desktop (please complete the following information):
Applicable to all desktops

Smartphone (please complete the following information):
Applicable to all mobiles

@rajat-gl rajat-gl added Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect labels Jan 23, 2020
@knsv
Copy link
Collaborator

knsv commented Jan 23, 2020

That was a seriously large diagram. Thanks for reporting this. Even if the diagram is to large it should not hand the page, better to refuse stating that the diagram is to large or similar.

@knsv knsv added Status: Approved Is ready to be worked on Topic: Platform and removed Status: Triage Needs to be verified, categorized, etc labels Jan 23, 2020
@knsv
Copy link
Collaborator

knsv commented Jan 25, 2020

Been looking at this one and it turns out that the parser is ok. I first thought that was the critical point but instead it was generating that large amount of nodes in the DOM when rendering the svg.

I see two ways of handling this. Either I limit the diagram on the text size or by the number of generated DOM elements.

The size method is more generic and can be done for all diagrams before the parsing is initiated. The other way is to make sure all diagrams keep track of the number of elements in the diagrams, this has to be done specificly for each diagram type.

I lean towards the size method as this one will be easier to maintain and more future proof. There will be a default max size and possibility to override that for when configuring mermaid by an integrator.

@knsv knsv self-assigned this Jan 25, 2020
@knsv
Copy link
Collaborator

knsv commented Jan 25, 2020

This fix will be included in 8.4.7

knsv added a commit that referenced this issue Jan 27, 2020
#1216 Fix for issue when mermaid freezes the browser tab due to large…
@github-actions github-actions bot locked and limited conversation to collaborators Jan 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Status: Approved Is ready to be worked on Type: Bug / Error Something isn't working or is incorrect
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants