LGTM2
On 3/21/23 3:01 AM, Noam Rosenthal wrote:
On Mon, Mar 20, 2023 at 8:13 PM Mason Freed <mas...@chromium.org> wrote:
On Mon, Mar 20, 2023 at 1:19 AM Noam Rosenthal
<nrosent...@chromium.org> wrote:
Voicing some concern about this API that I've raised before,
and perhaps I'm reading this wrong and it was addressed.
Think of CMS-style sites that embed user-generated HTML, like
Wikis (I worked on popups for wikipedia).
This HTML is usually sanitized to remove potentially malicious
tags (<script>, <object>) and also script-invoking events (on*
etc).
One of the things those content system have to do is make sure
that the embedded content is clearly separated from the
platform UI.
That is usually done with z-index & overflow. No matter how
you play around with your embedded content, if it is embedded
inside a stacking/overflow context, it can't go on top of the
platform UI.
By allowing showing/hiding top-layer elements without JS, we
break this contract and existing HTML sanitation-based
systems. Wiki and some other existing CMSs can resolve this by
special-casing popover attributes in their sanitizers, but I
wonder if it would break existing content systems that don't
have that privilege, don't know about it, or are not actively
developing their HTML sanitizer.
Note that this concern is only about the capability to open a
popup without JS.
Thanks for raising this issue. You and I discussed this several
months ago. I think my view is the same as before: using `z-index`
and `overflow` as some kind of security boundary is a bit fragile,
and not what those features were designed to do. There *is* a
platform API that *does* have this behavior as its official
contract: `<iframe>`.
<iframes> come with additional constraints. e.g. some of this embedded
HTML can position itself in the page (as long as it doesn't go "on
top" of other things), and you can't apply global CSS into iframes.
There's a reason people use embedded HTML rather than iframes for
certain use-cases, and stacking/overflow contexts gives some
confidence that the embedded HTML doesn't try to go on top of the
embedding UI.
As you mentioned, you already need to use a sanitizer to preserve
z-index boundary, since `dialog.showModal` or
`anyElement.requestFullscreen()` or even
`document.body.appendChild()` breaks out of it. And given that
sanitizers are a) required for this use case anyway, b) always
require upkeep to ensure they're filtering the right things, and
c) should be using allowlists or they're already broken, it seems
like that's the path forward for this type of CMS use case, right?
Probably the attribute that should be filtered is `popovertarget`,
to avoid the declarative invocation behavior.
Sanitizers are just one way to set a boundary for embedded HTML. The
other one is preventing JS using CSP.
Looking at the major sanitizers in use today (e.g. Github markdown,
Wiki HTML sanitizer) they use allowlists so this would not present a
problem for them.
I don't think this should be a blocker for this feature (which I'm
really excited about!) but I raised it to a wider audience because I
think we should stay aware of this issue. We're relaxing a very old
constraint here (albeit for good reasons).
--
You received this message because you are subscribed to the Google
Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send
an email to blink-dev+unsubscr...@chromium.org.
To view this discussion on the web visit
https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAJn%3DMYbVj4vVX92XaCCP1FQkBE5fjpWrZ2yHe2hz0rt%2BmhORyg%40mail.gmail.com
<https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAJn%3DMYbVj4vVX92XaCCP1FQkBE5fjpWrZ2yHe2hz0rt%2BmhORyg%40mail.gmail.com?utm_medium=email&utm_source=footer>.
--
You received this message because you are subscribed to the Google Groups
"blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-dev+unsubscr...@chromium.org.
To view this discussion on the web visit
https://groups.google.com/a/chromium.org/d/msgid/blink-dev/521f60c1-cd6f-fc61-0e36-d7e76dc512da%40chromium.org.
