GHSA-8xx9-69p8-7jp3 LiquidJS has a renderLimit DoS guard bypass via empty `{% for %}` body
SummaryThe `renderLimit` option — documented in `docs/source/tutorials/dos.md` as the mechanism that "mitigates this by limiting the time consumed by each render() call" — can be fully bypassed by a `{% for %}` (or `{% tablerow %}`) tag whose body is empty. The per-iteration time check is reached only when the body contains at least one template node, so a template like `{%- for i in (1..N) -%}{%- endfor -%}` iterates the full collection without ever consulting `renderLimit`. With a configured `renderLimit` of 50 ms, a single `parseAndRenderSync` call has been observed to consume **2.26 seconds** (~45× over the limit) and scales linearly with `N` up to `memoryLimit`, allowing a low-privileged template author to wedge an event-loop thread for an attacker-chosen duration.Details`Render.renderTemplates` is the single point at which `renderLimit` is consulted:The check at line 20 lives **inside** the `for (const tpl of templates)` body. When `templates.length === 0`, the loop body never executes, so the limiter is never consulted on that invocation.The `for` tag re-enters `renderTemplates` once per collection item with no independent time check:When `{%- for i in (1..N) -%}{%- endfor -%}` is parsed, `this.templates` is `[]`. Each of the `N` calls to `r.renderTemplates(this.templates, ctx, emitter)` therefore performs zero `renderLimit.check()` calls and zero template work — it just spins the JS-level `for` loop and the generator boilerplate. With `N = 30_000_000` this still costs ~2.26 s of CPU, and `N = 100_000_000` costs ~9.6 s, fully bypassing whatever wall-clock budget the integrator configured.The range expression itself is bounded only by `memoryLimit`:So the maximum bypass is governed by the (separate) `memoryLimit`, not by `renderLimit`. Integrators following the `docs/source/tutorials/dos.md` guidance — which positions `renderLimit` as the time-based defense — get no time-based defense at all on this code path.PoCReproduced against `liquidjs@10.25.7` (HEAD `34877950`):Scaling `N`:`N = 30_000_000` → 2255 ms (≈ 45× over the 50 ms limit)`N = 100_000_000` → 9581 ms (≈ 191× over the 50 ms limit)Time grows linearly with `N`, capped only by `memoryLimit` (default `Infinity`, so the only cap by default is process memory).ImpactAny liquidjs integrator who follows the upstream DoS guidance and sets a finite `renderLimit` to bound per-render CPU — typical for SaaS / multi-tenant environments where end users author templates (themes, email templates, snippets) — does not get the bound they configured. A single template submission can keep an event-loop thread busy for seconds, which on a Node.js server is sufficient to stall all in-flight requests on that worker. With a large enough range and a permissive `memoryLimit`, the wedge time is attacker-controlled. No data is exposed and no integrity is harmed; impact is availability only.Recommended FixMove the `renderLimit` check to a location that runs unconditionally per `renderTemplates` invocation, so a zero-template body still triggers it; alternatively (or additionally) have iteration tags that invoke `renderTemplates` per element check the limiter themselves once per iteration.And/or, defensively, in the iteration tags themselves so the guard cost is paid once per element rather than only at re-entry:The same hardening should be applied anywhere a tag drives an attacker-influenced loop count over a (potentially empty) `templates` array.
CVSS v3.1 score: 6.5