Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposed api: innerHTML setter that takes template strings #102

Closed
mozfreddyb opened this issue Jun 23, 2021 · 10 comments
Closed

proposed api: innerHTML setter that takes template strings #102

mozfreddyb opened this issue Jun 23, 2021 · 10 comments

Comments

@mozfreddyb
Copy link
Collaborator

(...)

given that we cannot distinguish that type from a string it's still unclear to me what that means. And even once we can distinguish it, what it would mean then.

To give more high-level description, Web Platform currently doesn't have a way to say developers are assigning static string or dynamic string.
That is,

elem.innerHTML = "<b>hello</b>";

AND

elem.innerHTML = "<b>" + location.hash.slice(1) + "</b>";

Are both string assignment (even though one is static and one is dynamic). What @koto suggested is to give a primitive to developers, where the platform can understand the notion of static string assignment. This primitive is important in the platform, because if we know something is static, it is safe to append without sanitization.

BTW, elem.setHTML.withLiterals`this_is_not_injected_for_sure` sounds better (remember, this is the thread to decide namings 😋)

Originally posted by @shhnjk in #100 (comment)

@mozfreddyb
Copy link
Collaborator Author

mozfreddyb commented Jun 23, 2021

I'm quoting @shhnjk's comment as this the most cohesive summary of what @koto suggested upthread. I hope you don't mind.

The idea is that we have an HTML-setter that is used as a tagged template string and can therefore distinguish literals from other strings and sanitize accordingly.

I think this needs to wait until v2, but happy to hear otherwise.

@annevk
Copy link
Collaborator

annevk commented Jun 23, 2021

I'd suggest elem.setRawHTML`...` for this. It seems like a reasonable addition that could be proposed directly to the HTML Standard.

@koto
Copy link

koto commented Jun 23, 2021

elem.setRawHTML...

Yes, that sounds good. I agree that has little to do with the sanitizer itself.

@otherdaniel
Copy link
Collaborator

Wouldn't this require some degree of language support?

I thought several related proposals were made to TC39, but none of them made it (yet?).

@koto
Copy link

koto commented Jun 23, 2021

The proposals were for a user-callable function. I think the existing machinery in ES allows us to implement this check already in a platform mechanism (not sure how elegant that is though, and whether it needs adjusting WebIDL).

Effectively one checks if the template object (first argument to the function) is already present in the [[Array]] slot of one of the entries in the current Realm's' [[TemplateMap]]. That assures that the call was at least a literal somewhere.

elem.setRawHTML`abc`

Note that "bypasses" are possible, like below, but still at some point the whole value was a literal in the code.

const literal = (foo=>foo)`bar`;
elem.setRawHTML(literal);

@domenic
Copy link

domenic commented Jul 13, 2021

Note that the correct form is setHTML(raw`foo`) not setRawHTML`foo`: tags should not have side effects, as they are supposed to only escape things. The side effect should be in a proper method that accepts the result of the tag.

@koto
Copy link

koto commented Jul 13, 2021

Thanks @domenic, I didn't realize that fag functions cannot have side effects (I know many in our codebases that do, though)!

Under this restriction, indeed setRawHtml`payload` is not possible. More so - I don't see any way for the platform to securely allow literals to be assigned to HTML that doesn't also require something like Trusted Types (value types that are immutable and vetted to be OK for a given purpose).

@koto
Copy link

koto commented Jul 15, 2021

Perhaps if the tag function returned a DocumentFragment (or threw on validation failure)?

Then node.replaceChildren(DocumentFragment.fromLiteral`foo<div>bar</div>`) could be a good replacement for node.innerHTML = "foo<div>bar</div>". The latter allows for dynamic payloads (and XSS in turn), but the former - not really.

@shhnjk
Copy link

shhnjk commented Jul 15, 2021

Perhaps if the tag function returned a DocumentFragment (or threw on validation failure)?

Then node.replaceChildren(DocumentFragment.fromLiteral`foo<div>bar</div>`) could be a good replacement for node.innerHTML = "foo<div>bar</div>". The latter allows for dynamic payloads (and XSS in turn), but the former - not really.

This sounds good, but can we also add this method to other Trusted Types? That is, TrustedHTML.fromLiteral, TrustedScript.fromLiteral, and TrustedScriptURL.fromLiteral?

@annevk
Copy link
Collaborator

annevk commented Oct 18, 2023

We ended up solving this in a different way with safe and unsafe methods. See also #196.

@annevk annevk closed this as not planned Won't fix, can't repro, duplicate, stale Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants