Align MathFilter limit with rest of Banzai

Asherah Connorrequested to merge
push-sorvlxrxwsnu into master

What does this MR do and why?

This limit was added in gitlab-org/security/gitlab!2088+ (staff-only; see 40b1b67a for the public commit) in response to Dos by Exploiting math feature on issue page. (#341832 - closed).

At that time, MathFilter worked very differently; see https://gitlab.com/gitlab-org/gitlab/-/blob/40b1b67a4c437304ab4646207cb5bbb420ce9ec8/lib/banzai/filter/math_filter.rb. Every single <code> was checked, its previous/next siblings enumerated, and on match, the previous and next siblings' inline content strings were permuted.

These are significant string copies: each sibling string is copied, and for consecutive math blocks with only text between, all in-between strings are copied twice! This easily lends itself to a DoS.

We no longer do anything of the sort: we laser target the applicable nodes, set a class, and optionally set another attribute. It is extremely cheap.

The current work done by the filter frankly no longer necessitates any limit, but to avoid doubt, we instead set a very high one.

Note that we have frontend limits in place to prevent degradation of the browsing experience: the restrictions removed in this MR related only to the backend timing out on render, which (per above) is no longer an issue.

/cc @reinaortega

Screenshots or screen recordings

Before After
image image

How to set up and validate locally

  1. Import a Markdown blob into your repository with a large number of math blocks. I've used an example given by a customer.
  2. View the blob: after 50 math blocks, they will no longer be rendered as math, and instead the raw source is shown as plain text.
  3. Check out this branch.
  4. To force the Markdown cache to become unsettled, edit the Markdown blob (add a newline or space somewhere, for example).
  5. The math blocks should now all be rendered, up to 1000.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Asherah Connor

Merge request reports