Skip to content

(deepsec) [BUG] DOMPurify hook is added on every sanitize call without removal #2230

@adriangohjw

Description

@adriangohjw

closes https://www.notion.so/opengov/isomer-other-dompurify-hook-leak-7e827d96fb-36e77dbba78881e4903ac38e6c3d7618

File: [packages/components/src/templates/next/components/complex/Iframe/Iframe.tsx](https://github.com/opengovsg/isomer/blob/main/packages/components/src/templates/next/components/complex/Iframe/Iframe.tsx#L37) (lines 37)
Project: isomer
Severity: BUG • Confidence: high • Slug: other-dompurify-hook-leak

Finding

Every call to getSanitizedIframeWithTitle (used by this Iframe component on each render) calls DOMPurify.addHook("beforeSanitizeElements", ...) (see packages/components/src/utils/getSanitizedIframeWithTitle.ts line 6) without ever calling DOMPurify.removeHook or removeAllHooks. Hooks accumulate in the singleton DOMPurify instance for the lifetime of the process. On a server-rendered page this means every Iframe render adds another hook that fires on every subsequent DOMPurify.sanitize call across the whole codebase. Functionally the latest hook’s title wins (because hooks run in insertion order and overwrite the title attribute), so the result is correct, but the memory/perf cost grows unboundedly and the hook will also run for any other future DOMPurify usage that includes iframes, applying a stale title from a previous render to those iframes. Security impact is minimal (the hook only mutates attributes on iframe elements, doesn’t introduce XSS), so this is a BUG rather than a vulnerability.

Recommendation

In getSanitizedIframeWithTitle, either (a) DOMPurify.removeAllHooks() before/after the sanitize call, (b) use a local DOMPurify instance via DOMPurify(window) scoped to this function, or (c) avoid the hook altogether by setting the title/etc. attributes on the returned DOM fragment AFTER sanitization rather than via a hook.

Revalidation

Verdict: true-positive

Confirmed. getSanitizedIframeWithTitle at packages/components/src/utils/getSanitizedIframeWithTitle.ts line 6 calls DOMPurify.addHook("beforeSanitizeElements", ...) unconditionally on every invocation. I grepped the entire packages/components directory for removeHook and removeAllHooks and found zero matches, confirming the hook is never removed. The Iframe component (line 37) calls this helper on every render. Since isomorphic-dompurify exports a shared singleton, hook callbacks accumulate for the process lifetime in SSR. Functionally the latest-added hook overwrites the title attribute last (so the rendered iframe gets the correct title), but: (a) memory grows linearly with renders, (b) every DOMPurify.sanitize call anywhere in the codebase that processes an iframe element will run all accumulated hooks, applying a stale title from a previous render. Security impact is minimal — the hook only sets benign attributes on iframe elements and DOMPurify’s allowed-tags list already restricts content to iframe — so BUG severity is appropriate. The proposed fixes (removeAllHooks, scoped DOMPurify instance, or post-sanitization attribute setting) are all valid.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions