Merge branch 'security-mermaid-rc-13-6' into 'master'

Fix mermaid resource consumption in GFM fields

See merge request gitlab-org/security/gitlab!1059

app/assets/javascripts/behaviors/markdown/render_mermaid.js

@@ -18,7 +18,13 @@ import { __, sprintf } from '~/locale';
 //
 
 // This is an arbitrary number; Can be iterated upon when suitable.
-const MAX_CHAR_LIMIT = 5000;
+const MAX_CHAR_LIMIT = 2000;
+// Max # of mermaid blocks that can be rendered in a page.
+const MAX_MERMAID_BLOCK_LIMIT = 50;
+// Keep a map of mermaid blocks we've already rendered.
+const elsProcessingMap = new WeakMap();
+let renderedMermaidBlocks = 0;
+
 let mermaidModule = {};
 
 function importMermaidModule() {
@@ -110,13 +116,22 @@ function renderMermaids($els) {
       let renderedChars = 0;
 
       $els.each((i, el) => {
+        // Skipping all the elements which we've already queued in requestIdleCallback
+        if (elsProcessingMap.has(el)) {
+          return;
+        }
+
         const { source } = fixElementSource(el);
         /**
-         * Restrict the rendering to a certain amount of character to
-         * prevent mermaidjs from hanging up the entire thread and
-         * causing a DoS.
+         * Restrict the rendering to a certain amount of character
+         * and mermaid blocks to prevent mermaidjs from hanging
+         * up the entire thread and causing a DoS.
          */
-        if ((source && source.length > MAX_CHAR_LIMIT) || renderedChars > MAX_CHAR_LIMIT) {
+        if (
+          (source && source.length > MAX_CHAR_LIMIT) ||
+          renderedChars > MAX_CHAR_LIMIT ||
+          renderedMermaidBlocks >= MAX_MERMAID_BLOCK_LIMIT
+        ) {
           const html = `
           <div class="alert gl-alert gl-alert-warning alert-dismissible lazy-render-mermaid-container js-lazy-render-mermaid-container fade show" role="alert">
             <div>
@@ -146,8 +161,13 @@ function renderMermaids($els) {
         }
 
         renderedChars += source.length;
+        renderedMermaidBlocks += 1;
+
+        const requestId = window.requestIdleCallback(() => {
+          renderMermaidEl(el);
+        });
 
-        renderMermaidEl(el);
+        elsProcessingMap.set(el, requestId);
       });
     })
     .catch(err => {

changelogs/unreleased/security-mermaid-rc-13-6.yml

+---
+title: Fix mermaid resource consumption in GFM fields
+merge_request:
+author:
+type: security

spec/features/markdown/mermaid_spec.rb

@@ -19,6 +19,9 @@ RSpec.describe 'Mermaid rendering', :js do
 
     visit project_issue_path(project, issue)
 
+    wait_for_requests
+    wait_for_mermaid
+
     %w[A B C D].each do |label|
       expect(page).to have_selector('svg text', text: label)
     end
@@ -39,6 +42,7 @@ RSpec.describe 'Mermaid rendering', :js do
     visit project_issue_path(project, issue)
 
     wait_for_requests
+    wait_for_mermaid
 
     expected = '<text style=""><tspan xml:space="preserve" dy="1em" x="1">Line 1</tspan><tspan xml:space="preserve" dy="1em" x="1">Line 2</tspan></text>'
     expect(page.html.scan(expected).count).to be(4)
@@ -65,6 +69,9 @@ RSpec.describe 'Mermaid rendering', :js do
 
     visit project_issue_path(project, issue)
 
+    wait_for_requests
+    wait_for_mermaid
+
     page.within('.description') do
       expect(page).to have_selector('svg')
       expect(page).to have_selector('pre.mermaid')
@@ -92,6 +99,9 @@ RSpec.describe 'Mermaid rendering', :js do
 
     visit project_issue_path(project, issue)
 
+    wait_for_requests
+    wait_for_mermaid
+
     page.within('.description') do
       page.find('summary').click
       svg = page.find('svg.mermaid')
@@ -118,6 +128,9 @@ RSpec.describe 'Mermaid rendering', :js do
 
     visit project_issue_path(project, issue)
 
+    wait_for_requests
+    wait_for_mermaid
+
     expect(page).to have_css('svg.mermaid[style*="max-width"][width="100%"]')
   end
 
@@ -147,6 +160,7 @@ RSpec.describe 'Mermaid rendering', :js do
     end
 
     wait_for_requests
+    wait_for_mermaid
 
     find('.js-lazy-render-mermaid').click
 
@@ -156,4 +170,55 @@ RSpec.describe 'Mermaid rendering', :js do
       expect(page).not_to have_selector('.js-lazy-render-mermaid-container')
     end
   end
+
+  it 'does not render more than 50 mermaid blocks', :js, quarantine: { issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/234081' } do
+    graph_edges = "A-->B;B-->A;"
+
+    description = <<~MERMAID
+    ```mermaid
+    graph LR
+    #{graph_edges}
+    ```
+    MERMAID
+
+    description *= 51
+
+    project = create(:project, :public)
+
+    issue = create(:issue, project: project, description: description)
+
+    visit project_issue_path(project, issue)
+
+    wait_for_requests
+    wait_for_mermaid
+
+    page.within('.description') do
+      expect(page).to have_selector('svg')
+
+      expect(page).to have_selector('.lazy-alert-shown')
+
+      expect(page).to have_selector('.js-lazy-render-mermaid-container')
+    end
+  end
+end
+
+def wait_for_mermaid
+  run_idle_callback = <<~RUN_IDLE_CALLBACK
+  window.requestIdleCallback(() => {
+    window.__CAPYBARA_IDLE_CALLBACK_EXEC__ = 1;
+  })
+  RUN_IDLE_CALLBACK
+
+  page.evaluate_script(run_idle_callback)
+
+  Timeout.timeout(Capybara.default_max_wait_time) do
+    loop until finished_rendering?
+  end
+end
+
+def finished_rendering?
+  check_idle_callback = <<~CHECK_IDLE_CALLBACK
+    window.__CAPYBARA_IDLE_CALLBACK_EXEC__
+  CHECK_IDLE_CALLBACK
+  page.evaluate_script(check_idle_callback) == 1
 end