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

Consider enforcing TT for custom attributes. #288

Closed
mikewest opened this issue Aug 26, 2020 · 4 comments
Closed

Consider enforcing TT for custom attributes. #288

mikewest opened this issue Aug 26, 2020 · 4 comments
Labels
future In consideration for the future releases of the API

Comments

@mikewest
Copy link
Member

mikewest commented Aug 26, 2020

Many modern frameworks use data- attributes for a number of purposes, up to and including script execution (see Script Gadgets for some examples). While the browser understands the purpose of built-in attributes, and can bind them to a Trusted Type for enforcement in setAttribute(...), et al, we can't do the same for data- attributes, as we don't understand their purpose.

In other parts of the platform, we've established naming conventions that we then imbue with meaning (e.g. __Secure- and __Host- cookie name prefixes). Perhaps doing the same here could be helpful. For example, we could extend the data- prefix to include type information (data-html-, data-script-, data-script-url-), and establish the same protections for those attributes as we would for their built-in equivalents (srcdoc, onclick, src). To spell this out a bit:

const el = document.createElement('div');

// These assignment might end up in `innerHTML`, and would be unsafe:
el.setAttribute('data-body', '<svg onload="alert(1)">');
el.dataset.body = '<svg onload="alert(1)">';


// The browser knows to treat these assignments as requiring a TrustedHTML object, and throw:
el.setAttribute('data-html-body', '<svg onload="alert(1)">');
el.dataset.htmlBody = '<svg onload="alert(1)">';

The information contained in this encoding might also be useful in other contexts. Sanitizers, for instance, could make different decisions about these attributes. See WICG/sanitizer-api#20 (comment), for instance.

Note: The prefix model proposed above is one spelling that makes sense to me. You could also imagine data-trustedhtml- or data-danger!!!-. Or suffixes! Or any other spelling that makes you happier. :)

@shhnjk
Copy link
Member

shhnjk commented Aug 31, 2020

I didn't get the benefit of having TT enforcement on these attributes. If anything from custom attribute would end up in innerHTML, then TT check would happen at that point, and it wouldn't make much difference if we do this earlier or later?

Could you elaborate bit more on benefits of having this?

@mikewest
Copy link
Member Author

mikewest commented Sep 1, 2020

The examples in the original comment seem like cases in which TT enforcement would be helpful. Adding attributes to an element directly can result in script execution when the element is attached to the page. Some attributes (like onload) are well-understood by the user agent, and we enforce TT upon them. Custom attributes might have meaning to a framework that's opaque to the browser; the suggestion here is that browsers could enforce upon them as well at assignment-time if we understood their meaning.

Standardizing on a naming convention would also make crafting a TT policy simpler for the innerHTML case you point to, since sanitizers would have a framework-agnostic understanding of the risk of a given attribute based on its name.

@Siegrift
Copy link
Contributor

Shouldn't this be a concern of the framework/library?

@koto
Copy link
Member

koto commented Jan 19, 2024

Let's postpone this until there's broader community requests to make sure we understand the use case more.

@koto koto closed this as completed Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
future In consideration for the future releases of the API
Projects
None yet
Development

No branches or pull requests

4 participants