Strip template elements from the HTML block preview (#23866)
Follow-up to a code scanner alert on `HtmlNodeView`. The alert itself is
not a live vulnerability, but auditing the hand-rolled sanitizer behind
it turned up one gap worth closing.
## What I checked
`sanitizeHtmlPreview` is a blocklist sanitizer feeding
`dangerouslySetInnerHTML` in the editor, so I ran ~30 payloads through
the real implementation in headless Chromium, inserting the sanitized
output into a live document. A deliberately unsanitized control payload
fired, so the harness was actually detecting execution.
Nothing executed. The parser-differential classes it already survives:
`noscript`, `noembed`, `noframes`, `xmp`, `listing`, `title` and
`textarea` raw-text handling; `svg`/`math`/`mglyph`/`foreignObject`
namespace confusion; comment breakouts; table foster parenting; `<body
onload>`; `<iframe srcdoc>`; `data:text/html`; and `javascript:` with
entity and control-character obfuscation.
## The gap
`document.body.querySelectorAll('*')` does not descend into a
`<template>`'s content, which lives in a separate document fragment. So
this:
```html
<template>``<img src=x onerror="alert(1)">``</template>
```
came back out of the sanitizer with its handler intact.
It does not execute as currently used — template content is inert when
assigned through `innerHTML`, and I confirmed that. But a live event
handler sitting inside a string the sanitizer just declared clean is a
footgun for anything that later clones, re-parses or forwards it.
The outbound email sanitizer already drops template content, so blocking
the element here also stops the preview from showing something the sent
email would not contain.
## Scope
One word in the blocklist plus tests. Inline styles are deliberately
untouched, since dropping them would make the preview diverge from the
rendered email.
An earlier revision also added `contain: paint` to the preview, to stop
a block from painting over the editor chrome. That was dropped after
testing showed it clips box-shadows bleeding past the block edge and
negative-margin full-bleed layouts, both normal in email design. Details
in the review thread. The overlay it guarded against needs an
authenticated member who can already edit the template, which does not
justify constraining newsletter design.
## Testing
Re-ran the full payload corpus with the new blocklist. The template
payload's handler is gone, every other result is byte-identical, and the
control still fires. The two new unit tests were verified to pass under
jsdom, the environment jest actually uses.
This commit is contained in:
+13
@@ -53,6 +53,19 @@ describe('sanitizeHtmlPreview', () => {
|
||||
).toContain('data:image/png');
|
||||
});
|
||||
|
||||
it('should remove templates instead of leaving their handlers unreachable', () => {
|
||||
expect(
|
||||
sanitizeHtmlPreview(
|
||||
'<template><img src=x onerror="alert(1)"></template>',
|
||||
),
|
||||
).toBe('');
|
||||
expect(
|
||||
sanitizeHtmlPreview(
|
||||
'<div><template><img src=x onerror="alert(1)"></template></div>',
|
||||
),
|
||||
).toBe('<div></div>');
|
||||
});
|
||||
|
||||
it('should strip srcdoc and formaction attributes', () => {
|
||||
expect(
|
||||
sanitizeHtmlPreview('<div srcdoc="<script>x</script>">a</div>'),
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
const BLOCKED_ELEMENT_SELECTOR =
|
||||
'script, iframe, frame, object, embed, link, meta, base, style, svg, math';
|
||||
'script, iframe, frame, object, embed, link, meta, base, style, svg, math, template';
|
||||
|
||||
const URL_ATTRIBUTE_NAMES = ['href', 'src', 'xlink:href', 'action'];
|
||||
|
||||
|
||||
Reference in New Issue
Block a user