Re: [whatwg/dom] Declarative Shadow DOM (#831)

It's become clear that there are actually _two_ separate XSS issues under discussion. Let's consider the two security issues separately. There's what I'd call the "client-side XSS" (CSX) and the "server-side XSS" (SSX).

The CSX issue is pretty tricky, and occurs when all of these conditions hold:

1. The browser understands shadow DOM
2. The client-side sanitizer uses the browser's parser
3. The sanitizer returns raw DOM nodes
4. The code to be sanitized includes closed shadow roots

The SSX issue is much simpler to understand:

1. Today, `<template>` elements are inert
2. Some server-side sanitizer might assume that `<template>` elements will always be inert, and not strip them in any way at all.
3. When declarative shadow DOM is enabled, `<template shadowroot>` would not be inert, breaking the assumptions of the server-side sanitizer.

A number of us on this thread (including, at least, me!) incorrectly thought that the CSX issue, which is tricky to understand, was just the SSX issue, which is easy to understand.

### Server-side sanitizers simply must sanitize `<template>`s

There are browsers in depressingly common use today that don't support the `<template>` tag. https://caniuse.com/template IE11 doesn't support it, Opera Mini doesn't support it, and Safari 7 doesn't support it. Those appear to be the most popular legacy browsers in modern use that don't support `<template>`, but if you're designing a sanitizer, even if you "don't support IE11," it's not OK to simply assume that _any_ client you engage with will support `<template>`.

Therefore, for security purposes, it is not acceptable to skip sanitizing dangerous code like `<script>` and `onmousemove` attributes inside `<template>`. At the very least, that will pwn your IE11, Opera Mini, and Safari 7 users.

Since legacy browsers exist, it is simply incorrect for a server-side sanitizer to assume that `<template>` will necessarily be inert. Any sanitizer that assumes that is buggy/wrong _today._

Luckily, even the most naive server-side sanitizers will automatically do the right thing if they just walk through `<template>` like an unknown element. They'll be even better off if they do what all of the popular ones seem to do and just delete `<template>` altogether during sanitization.

Thus, writing a server-side sanitizer with this _particular_ bug is actually rather hard to do; if anyone did it, they'd be buggy right now, even without declarative shadow DOM, so that might give someone even more cause to catch it.

I think it should be a non-goal for declarative shadow DOM to be compatible with _buggy_ sanitizers, and so we don't need to consider further changes to address the SSX issue (and _especially_ not if fixing this would require switching to `<shadowroot>`, which could introduce _other_ security issues in its browser implementation).

I don't think any changes should be considered to address the SSX issue unless someone were to provide evidence that sanitizers with this particular SSX bug were common today (or at least easy to write accidentally).

### An opt-in mechanism could make sense for CSX

In the CSX case, contradicting my earlier "footgun" argument, I think there _is_ a possible opt-in mechanism we can use, because the page isn't trying to sanitize _itself_ on the client side, (how could it possibly?!) … a client-side sanitizer must be sanitizing some _other_ input.

When using `DOMParser`, an `options` argument could be added to its constructor, enabling support for closed declarative shadow roots. (`innerHTML` doesn't have a constructor, but I can imagine some pragma property on the `Element` object or something, `div.security.allowClosedDeclarativeShadowRoots = true; div.innerHTML = dirty`.)

As I'm imagining it, the browser's parser would just delete closed shadow roots by default, but when the pragma is set, closed shadow roots would appear as DOM nodes in the parsed result. The sanitizer would then have to make sure to `importNode` when enabling this pragma.

Note also that this pragma could be restricted to _closed_ declarative shadow roots, reducing its impact on usability.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/dom/issues/831#issuecomment-716987056