Skip to content

fix: align fallback checks with serialized tag names - #44

Open
Adyej999 wants to merge 1 commit into
angular:mainfrom
Adyej999:fix/serialized-fallback-ancestor-identity
Open

fix: align fallback checks with serialized tag names#44
Adyej999 wants to merge 1 commit into
angular:mainfrom
Adyej999:fix/serialized-fallback-ancestor-identity

Conversation

@Adyej999

@Adyej999 Adyej999 commented Sep 7, 2026

Copy link
Copy Markdown

PR Checklist

  • The commit message follows the project commit message guidelines
  • Tests for the changes have been added
  • The full repository test suite passes

PR Type

  • Bugfix
  • Security hardening
  • Feature
  • Code style update
  • Refactoring
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other

What is the current behavior?

Domino uses different element identities when determining whether an ancestor is a fallback raw-content element and when serializing that same element.

fallbackRawContentTags() currently only recognizes fallback ancestors in the HTML namespace:

if (
  node.namespaceURI === NAMESPACE.HTML &&
  hasRawContentFallback[node.tagName]
) {
  tags.push(node.localName);
}

However, serializeOne() serializes HTML, SVG, and MathML elements using localName:

var tagname =
  (html || ns === NAMESPACE.SVG || ns === NAMESPACE.MATHML)
    ? kid.localName
    : kid.tagName;

This means an element can be omitted from the fallback raw-content ancestor check while still being emitted with a fallback raw-content tag name such as noembed or iframe.

For example:

const fallback = document.createElementNS(
  'http://www.w3.org/2000/svg',
  'noembed',
);

is serialized as:

<noembed>

but was not previously recognized as a noembed ancestor by fallbackRawContentTags().

This affects the existing ProcessingInstruction closing-tag protection.

Given:

const pi = document.createProcessingInstruction(
  'x',
  '</noembed ',
);

followed by a sibling:

const img = document.createElement('img');
img.setAttribute('src', 'x');
img.setAttribute('onerror', 'alert(1)');

the unpatched serializer can produce:

<noembed><?x </noembed ?><img src="x" onerror="alert(1)"></noembed>

When the serialized SSR output is parsed as HTML, the matching </noembed sequence terminates the fallback raw-content context and the following sibling is parsed as active HTML.

What is the new behavior?

Element-name selection is centralized in a shared helper:

function serializedTagName(node) {
  var ns = node.namespaceURI;
  return (
    ns === NAMESPACE.HTML ||
    ns === NAMESPACE.SVG ||
    ns === NAMESPACE.MATHML
  )
    ? node.localName
    : node.tagName;
}

serializeOne() uses the same helper when determining the emitted element name:

var tagname = serializedTagName(kid);

and fallbackRawContentTags() uses that identity for fallback ancestor detection:

const tagname = serializedTagName(node);

if (
  tagname &&
  hasRawContentFallback[tagname.toUpperCase()]
) {
  tags.push(tagname);
}

As a result, an ancestor that serializes with a fallback raw-content tag name is also recognized by the existing closing-tag protection.

For the same SVG noembed example, serialization becomes:

<noembed><?x &lt;/noembed ?><img src="x" onerror="alert(1)"></noembed>

The matching closing-tag prefix is escaped and therefore does not terminate the fallback raw-content context.

Security impact

This keeps fallback raw-content ancestor detection consistent with the element identity used in serialized HTML.

Without this consistency, namespace or qualified-name representations can bypass the existing ProcessingInstruction ancestor-closing-tag protection while still producing the corresponding fallback tag name in the serialized response.

The demonstrated path is:

namespace / qualified-name fallback element
        ↓
ancestor check misses serialized fallback name
        ↓
ProcessingInstruction contains matching closing-tag prefix
        ↓
SSR serialization
        ↓
browser reparses the response as HTML
        ↓
fallback raw-content element closes early
        ↓
following sibling becomes active HTML
        ↓
JavaScript execution

This behavior was reproduced directly in Chromium against the Angular-pinned Domino revision.

Relation to GHSA-j3r3-mxqp-r2p4

GHSA-j3r3-mxqp-r2p4 introduced escaping for matching fallback raw-content ancestor closing tags in ProcessingInstruction data.

That protection depends on fallbackRawContentTags() correctly identifying the relevant serialized ancestor.

This change does not alter the ProcessingInstruction escaping mechanism itself. Instead, it makes the ancestor identity used by that protection consistent with the identity used by element serialization.

The existing escaping mechanism is therefore reused once the serialized fallback ancestor is identified correctly.

Does this PR introduce a breaking change?

  • Yes
  • No

For normal element serialization, this only centralizes the existing tag-name selection into a shared helper and does not change which tag name is emitted.

The intentional behavior change is limited to recognizing additional serialized fallback ancestors in the existing security-sensitive escaping paths.

Tests

A regression test was added for:

  • SVG noembed;
  • MathML noembed;
  • SVG iframe;
  • qualified-name HTML iframe.

The regression uses the executable sibling shape:

const pi = document.createProcessingInstruction(
  'x',
  '</noembed ',
);

const img = document.createElement('img');
img.setAttribute('src', 'x');
img.setAttribute('onerror', 'alert(1)');

The regression was first verified against the unmodified Angular-pinned Domino revision:

7df65450b8331278010e44eed0ef32225b07d203

Before this change, the regression fails because the matching fallback closing-tag prefix is not escaped.

With this change applied:

targeted regression: 1 passing
existing PI regression: 1 passing
XSS suite: 45 passing
full repository suite: 2179 passing

The following checks were run locally:

pnpm exec mocha test/xss.js \
  --grep "fallbackRawTextProcessingInstructionUsesSerializedAncestorName"

pnpm exec mocha test/xss.js \
  --grep "fallbackRawTextProcessingInstructionEscapesAncestorClosingTag"

pnpm exec mocha test/xss.js

pnpm test

git diff --check

Other information

The browser portion was also validated directly in Chromium against the exact unmodified Angular-pinned Domino revision:

7df65450b8331278010e44eed0ef32225b07d203

Before this change, all tested representations resulted in JavaScript execution:

SVG noembed             -> dialog=true
MathML noembed          -> dialog=true
SVG iframe              -> dialog=true
qualified-name iframe   -> dialog=true

For example, the unpatched SVG noembed case serialized as:

<noembed><?x </noembed ?><img
  src="x"
  onerror="alert('DOMINO_NAMESPACE_XSS:svg-noembed')"
></noembed>

and Chromium displayed the expected dialog.

With this PR applied, the same case serializes with the matching closing-tag prefix escaped:

<noembed><?x &lt;/noembed ?><img
  src="x"
  onerror="alert('DOMINO_NAMESPACE_XSS:svg-noembed')"
></noembed>

and Chromium does not execute the sibling handler.

The same before/after behavior was verified for all four regression representations.

@google-cla

google-cla Bot commented Sep 7, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@Adyej999
Adyej999 force-pushed the fix/serialized-fallback-ancestor-identity branch from 519e9a0 to b8eded4 Compare September 7, 2026 20:20
@Adyej999
Adyej999 force-pushed the fix/serialized-fallback-ancestor-identity branch from b8eded4 to 82ac580 Compare September 7, 2026 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant