Enhancement: Automatic Isolation of Hook Contexts in DOMPurify to Prevent State Leakage Between Sanitizations

[DrBronsy]

This issue proposes a [bug, feature] which...

Background & Context

This issue is observed when abstracting DOMPurify's sanitization process into a utility function. Hooks added via DOMPurify.addHook seem to inadvertently maintain state across multiple invocations of the sanitization function within the same runtime, leading to instances where parameters from prior sanitization calls are passed to hooks in subsequent calls.

Bug

Input

Consider HTML content that is provided to a sanitization utility function leveraging DOMPurify.

function sanitizeContent(html) {
  // Hook to perform a custom operation on the HTML content
  DOMPurify.addHook('uponSanitizeElement', (node, data, config) => {
    // Hook implementation...
  });

  const cleanHTML = DOMPurify.sanitize(html);
  // Perform additional operations if necessary...
  return cleanHTML;
} 

Repeated calls to sanitizeContent with different HTML inputs are made during the application's execution.

Given output

Despite expecting each sanitization process to be independent, we observe that hooks are receiving parameters from previous function executions, indicating that context is persistently shared or leaked between calls. The output given by DOMPurify.

Expected output

Each invocation of the sanitization utility function should result in hooks being executed in an isolated context, with no data bleeding from previous calls. Hooks should receive only the current parameters relevant to the specific DOMPurify.sanitize invocation they are associated with.

To temporarily address this issue:

function sanitizeContent(html) {
  // Clear all hooks before adding new ones to ensure a clean slate for hook execution
  DOMPurify.removeAllHooks();

  // Hook to perform a custom operation on the HTML content
  DOMPurify.addHook('uponSanitizeElement', (node, data, config) => {
    // Hook implementation...
  });

  const cleanHTML = DOMPurify.sanitize(html);
  // Perform additional operations if necessary...
  return cleanHTML;
}

Using DOMPurify.removeAllHooks() prior to adding hooks and invoking DOMPurify.sanitize ensures that previous hook context is not affecting the current sanitization call. This approach, while effective, is not an ideal or permanent solution.

Feature

While the temporary workaround using DOMPurify.removeAllHooks() does prevent the retention of context between hook invocations, the necessity to manually invoke this cleanup function introduces both additional performance overhead and potential for developer oversight.

[cure53]

Hey there, thanks for filing this. I do wonder however whether this is a bug or rather an oversight in the documentation.

If we fix this now, and make sure that hooks will be detached automatically after one sanitization round, this will likely destroy existing implementations. So, would it not be better to update the documentation and state more clearly that hooks are sticky and need to be removed if one wants to get rid of them?

[cure53]

Closing this for now as un-actionable