Allow to render TrustedHTML objects similar to SafeString

[crypto]

Summary

Currently TrustedHTML objects converted to strings during rendering before hitting DOM API that means all HTML insertions handled by default policy. That prevents us from migrating to Trusted Types. This change allows to render TrustedHTML objects similar to SafeString.

Testing done

ok 2382 [integration] jit :: TrustedHTML > renders TrustedHTML similar to SafeString
ok 2383 [integration] jit :: TrustedHTML > renders TrustedHTML in attribute context as string
1..2686
# pass 2685
# skip 1
# todo 0
# fail 0

[NullVoxPopuli]

Thanks for submitting this, this is neat!

[crypto]

Nice, "All checks have passed"

[NullVoxPopuli]

Of note,

• TrustedHTML is (at this time) not supported by FireFox and Safari
  • Compatibility: https://developer.mozilla.org/en-US/docs/Web/API/TrustedHTML#browser_compatibility
  • Polyfill: https://github.com/w3c/trusted-types
  • Spec (currently DRAFT): https://w3c.github.io/trusted-types/dist/spec/
    • Mozilla has committed to implement:
      • https://www.theregister.com/2023/12/21/mozilla_decides_trusted_types_is/
      • Trusted Types mozilla/standards-positions#20 (comment)

[NullVoxPopuli]

this is a good way to progressively allow the feature for browsers that have it and not break in browsers that don't

[chancancode]

This cast seems suspect

[chancancode]

I would think, if we made it here, we should have already checked and unwrapped the value?

[crypto]

We can modify whole chain of functions to accept string | TrustedHTML type, but I thought we should keep it when decide to fully migrate Glimmer to Trusted Types.

[crypto]

I've added TrustedHTML for appendHTML methods, let's see if we have any issues

[crypto]

I think it goes down to
@simple-dom/interface that doesn't support Trusted Types

glimmer-vm/packages/@glimmer/runtime/lib/dom/operations.ts

Line 69 in 1cb493b

insertHTMLBefore(parent: SimpleElement, nextSibling: Nullable<SimpleNode>, html: string): Bounds {

[chancancode]

Yes, we definitely still have to plumb this through to simple-dom. I am actually not sure why TypeScript is not catching the TypeError, but essentially the problem is you changed the interface without changing the implementations, so for example, in places like these, it's all going to be problematic:

https://github.com/glimmerjs/glimmer-vm/blob/main/packages/%40glimmer/runtime/lib/dom/operations.ts#L69-L74

glimmer-vm/packages/@glimmer/node/lib/node-dom-helper.ts

Lines 23 to 25 in 1cb493b

	html: string
	): Bounds {
	let raw = this.document.createRawHTMLSection!(html);

The code is a bit odd, because the primary implementation of insertHTMLBefore is on a class that doesn't claim to implement the interface you modified, so no type error there. But this extends and implement it, so it should be caught there, but it wasn't: https://github.com/glimmerjs/glimmer-vm/blob/main/packages/%40glimmer/runtime/lib/dom/api.ts#L18

Anyway missing type errors aside, there are some real, a bit tedious but solvable problems, there are also a few more things to think through around attributes (and then the grunt work to actually plumb it through, get simple-dom released, etc), so it may be a while before we can land this.

[crypto]

I've changed it back to not change interface of insertHTMLBefore.

[crypto]

If you think it will take a while, I suggest to taking small steps and in this PR just stringify TrusteHTML, but render it as HTML.

[crypto]

more things to think through around attributes

Let me explain why I'm against error on rendering TrustedHTML in attribute context.

I've tested LinkedIn where we use SafeString in attribute context, almost everywhere. It's related to our i18n system. Every i18n string is like small HTML template where all params HTML encoded, not every i18n string contains HTML tags, but potentially every string can contain HTML entities, like  . For this reason i18n helper uses htmlSafe for every i18n string. And obviously same SafeStrings used in attribute context. If we are going to replace SafeString with TrustedHTML it will not change anything. Yes, we have some rare double encoding issues, but throwing errors would completely prevent migration to TrustedHTML.

Another point is DOM API, some attributes require TrustedHTML to work, for example iframe.srcdoc

trustedTypes.getAttributeType('iframe', 'srcdoc') // TrustedHTML

https://w3c.github.io/trusted-types/dist/spec/#validate-attribute-mutation
For this reason we should keep TrusteHTML and other trusted types unmodified for setAttribute, decoding can cause an injection, stringifying can cause default policy triggering.
https://developer.mozilla.org/en-US/docs/Web/API/TrustedTypePolicyFactory/getAttributeType

I would recommend to keep same behavior as SafeString, and in best case don't stringify TrustedHTML object.

[NullVoxPopuli]

Thanks for walking through those use cases. That's helpful, and I'm fine with not throwing

[chancancode]

I discussed this with @wycats and here is a summary of the things we discussed.

In general:

• It's great that the platform feature is gaining traction and it probably supersedes the Ember side RFCs
• Indeed eventually this can be something that the browsers handle for us and we won't have to worry about it, some day we can deprecate SafeString
• That being said, it's probably still early/premature to RFC the SafeString deprecation or even recommend using trusted types to users generally, as it is only implemented by one browser and still going through changes
• But it makes sense to unblock (not get in the way of) those who are adopting the feature early

Research:

• The prevailing consensus is that as expression is not a thing anymore, CSS is no longer considered an injection (XSS) vector, but it is still very much an exfiltration vector
• The current proposal decided that for v1, it will only cover XSS, and does not currently do anything for exfiltration and other vectors
• It is planned that the feature can be expanded to cover other surfaces down the road with additional CSP opt-ins

For content position:

• It makes sense to allow {{{this.someTrustedHTML}}}
• To make that work, it will require plumbing it through to @simple-dom/interfaces:
  • Introduce a branded/opaque types for TrustedHTML, and any other trusted types we ended up using
  • insertAdjacentHTML needs to accept TrustedHTML
  • Introduce a SimpleTrustedTypes – a subset interface of TrustedTypePolicyFactory with the corresponding is* methods
  • Implementations need to be updated to match (specifically, SSR/rehydration) to account for those
• One additional challenge is that we currently we assume we can just pass around the SimpleDocument and have all the DOM APIs we need, with this change that is no longer the case, we need to somehow be able to accept and pass around SimpleTrustedTypes as well
  • We need to figure out how that is supposed to be configured (both here and in Ember)
  • In particular, that is relevant for fastboot/SSR, if we don't do this correctly all your {{{this.someTrustedHTML}}} will ended up being escaped

For attributes:

• Because the trusted types stringifies into their underlying values, for the most part, this already "work"
• ...except in the few cases where the v1 feature currently cares, i.e. getAttributeType(...) !== null, or getPropertyType(...) !== null
• For these to really work, similarly we need to plumb the logic to @simple-dom/interfaces:
  • We need to add getAttributeType() and getPropertyType() to SimpleTrustedTypes
  • setAttribute/setAttributeNS needs to accept the trusted types
  • Implementations need to be updated to match (specifically, SSR/rehydration) to account for those
• We have similar logic here, we can consider deferring to SimpleTrustedTypes.getAttributeType(), however
  • There may be cases where we don't currently require a SafeString but getAttributeType() returns non-null, these are arguably cases that are currently missed but it may be too much of a breaking change to just start requiring them
  • Conversely, we currently cover a wider surface than the V1 API: for example, we have a warning for styles={{...}} that can be silenced with a SafeString
    • We are not going to allow TrustedHTML to bypass the warning – the V1 API does not intend to provide any protection against the exfiltration vectors, if we allow TrustedHTML to bypass the style warning, we are conferring additional meaning to the type that the spec does not intend
    • If the spec expands to cover the exfiltration vectors, it may (and I would argue likely) introduce/require new trusted types (e.g. TrustedCSS) for these positions, so it would actually be incorrect to encourage developers to use TrustedHTML for this purpose today
    • So, TrustedHTML is not going to be a complete replacement of SafeString today. There are cases where you currently need a SafeString today that you can't simply replace with a TrustedHTML (or any of the other trusted types)

So overall, we think this is certainly a worthwhile feature to pursue, it's a bit of a project to get it implemented correctly, but a fairly bounded one.

If you think it would help enough to land a version where we simply stringify, and if you think that helps enough with your use case (keep in mind that it will still trigger CSP warnings because it won't make it all the way to the DOM APIs), we can certainly explore that. However, I'm not sure that would be much less work than doing everything correctly – the PR as it stands is still problematic, because we can't just rely on accessing the trusted types on the global object in SSR/fastboot, and if we are going to fix that we may as well do everything else?

Does LinkedIn not need this to work in SSR/fastboot?

[crypto]

If you think it would help enough to land a version where we simply stringify, and if you think that helps enough with your use case

It will take significant amount of time until we will be able to enable trusted-types CSP, but replacement SafeString with TrustedHTML already will be hugely beneficial in preventing HTML injections for one simple reason – developers no longer will require to call htmlSafe manually, because systems that not dependent on Ember.js could return TrustedHTML objects instead of strings. To summarize, migration to TrustedHTML will take time, and simple rendering it similar to SafeString would help a lot even if we couldn't utilize CSP.

the PR as it stands is still problematic, because we can't just rely on accessing the trusted types on the global object in SSR/fastboot

Internally we will be using object similar to SafeString or Trusted Types polyfill, I don't see issues with SSR. We will make sure that it works.

[chancancode]

Internally we will be using object similar to SafeString or Trusted Types polyfill, I don't see issues with SSR. We will make sure that it works.

I don't think you can. FastBoot/SSR runs in a sandboxed environment, so as it stand, what would happen is that there will be no window.TrustedType in SSR, and isHTML() will always return false, and so if you try to render {{this.someTrustedHTML}} there it will get escaped contrary to what you want.

[NullVoxPopuli]

in fastboot's sandboxed environment, you could provide an implementation for TrustedHTML, yeah?

is this feasible? or not worth it? 🤷

./config/fastboot.js
module.exports = function () {
  return {
    buildSandboxGlobals(defaultGlobals) {
      return {
        ...defaultGlobals,
        fetch,
        AbortController,
        ReadableStream: typeof ReadableStream !== 'undefined' 
            ? ReadableStream 
            : require('node:stream/web').ReadableStream,
        WritableStream: typeof WritableStream !== 'undefined'
            ? WritableStream
            : require('node:stream/web').WritableStream,
        TransformStream: typeof TransformStream !== 'undefined'
            ? TransformStream
            : require('node:stream/web').TransformStream,
        Headers: typeof Headers !== 'undefined' ? Headers : undefined,
        // custom stuff here
        TrustedHTML: YourImplementationHere       
      }
    }
  }
};

[crypto]

I just planned to use this code to generate TrustedHTML objects:

let createTrustedHTML: (s: string) => TrustedHTML;
let isTrustedHTML: (s: unknown) => boolean;
const trustedTypes = window.trustedTypes;
if (trustedTypes?.createPolicy && trustedTypes?.isHTML) {
  const policy = trustedTypes.createPolicy('jSecure', {
    createHTML: (s: string) => s
  });
  createTrustedHTML = (s: string) => policy.createHTML(s);
  isTrustedHTML = (value: unknown) => trustedTypes.isHTML(value);
} else {
  // if Trusted Types are not available, create object similar to Ember.js SafeString
  class HTML extends String {
    toHTML(): string {
      return this.toString();
    }
  }
  createTrustedHTML = (s: string) => new HTML(s) as unknown as TrustedHTML;

  isTrustedHTML = (value: unknown): boolean => {
    return (
      value !== null &&
      typeof value === 'object' &&
      'toHTML' in value &&
      typeof (value as HTML).toHTML === 'function'
    );
  };
}

[chancancode]

It think it should probably get passed here: https://github.com/ember-fastboot/ember-cli-fastboot/blob/242683f4137ca3dbcd53fdc0d8cc65f4055e3b57/packages/fastboot/src/ember-app.js#L362-L374

Along side the document: new SimpleDOM.Document() would be trustedTypes: { ... }

[chancancode]

is this feasible? or not worth it? 🤷

It's not that different from the plan I laid out, the main difference is that we would only define a strict subset in Simple.TrustedType, so you wouldn't have be implement the parts that we don't care about, and it won't force you to assign that to the global, because your polyfill may be incomplete, may not work with other libraries, etc.

Which is the same reason why you don't just set document to new Simple.Document() in that hook, because Simple.Document is missing a lot of things that are on the real Document, and if you make it globally available, it could break other code/library that tries to feature-detect document and do stuff with it.

In this case, you only really need to implement { isHTML(obj) { return myCustomCheck(obj); } }, but if you assign that to globalThis.trustedTypes = ... other code (that you probably didn't write) will see that and assume it's in an environment where it could freely use all the APIs usually available there.

[crypto]

Asking some experts about CSS exfiltration, I believe style attribute is not that bad. And htmlSafe doesn't provide security benefits there.
https://twitter.com/shafigullin/status/1761166166687617133
https://portswigger.net/research/blind-css-exfiltration

[chancancode]

That would be a pretty strong claim with a pretty high bar to clear, and there are additional considerations for client side JS apps.

Off the top of my head, a scenario would be that you have style={{...}} on a conditionally rendered element, then you can use a background image url to determine the fact that the element was rendered. At minimum, that could convey the fact that a e-mail/message was opened/read, or inferring what kind of access a user has, etc.

Besides exfiltration, it is also a phishing vector, e.g. re-positioning a comment/message box directly over a password field, hiding important warning text, that kind of stuff.

We can certainly discuss whether we originally meant to protect users against that kind of threat, or was it just meant for XSS because expressions were a thing back then. My recollection was that it was for both, and it was added in response to (or at least around the same time) industry experts calling out this kind of vectors in Angular/other frameworks. But in any case, that is not a small change that we are just going to make in a PR without an RFC.

[crypto]

I'm checking Discord conversation
https://discord.com/channels/480462759797063690/480502165434138624/1209976708082696222

but also, CSP would also have blocked innerHTML = "<script>...</script>

Inline script inserted with innerHTML doesn't work in any browser: innerHTML = "<script>...</script>

I guess you meant innerHTML = "<style>...</style>

For this specific reason CSP have additional directives:
style-src-elem
style-src-attr

Besides exfiltration, it is also a phishing vector, e.g. re-positioning a comment/message box directly over a password field, hiding important warning text, that kind of stuff.

I absolutely agree with all that, but these issues happen during HTML injection e.g. when attacker bypassed HTML sanitizer or able to inject attribute. Injection in style attribute itself is very rare, and that fact that developer warned about usage of htmlSafe will not prevent it.

Gareth replied: https://twitter.com/garethheyes/status/1761176827492552748

Like you said imports and attribute selectors don't work so exfiltration would be extremely difficult. Variables do work in inline styles and transitions/animations so maybe you could abuse complex existing styles to trigger requests.

[wycats]

Some big-picture thoughts:

The Status Quo

Today, Ember attempts to address certain potential security risks via the htmlSafe mechanism.

XSS Mitigations

The recommended approach for XSS is:

1. If at all possible, insert user content into the page using a template, which constructs HTML by creating DOM nodes rather than through string concatenation. This avoids XSS hazards caused by malicious user content using HTML characters (like > or ") to escape the expected HTML context.
2. If it's necessary to create an HTML string that contains user content, concatenate your own literals (e.g. <span>) with escaped user content (escapeExpression(userContent)). Then, mark the entire string as htmlSafe, which will allow it to be inserted into content positions in a template.

This approach maps onto security best practices that predate trusted types. It's not a confusion-free approach, but it's straight-forward once you understand what's going on, and it's pretty straight-forward for npm packages to use the approach to create helpers that produce safe HTML.

Exfiltration and Clickjacking

Ember uses the htmlSafe mechanism for another purpose: to mark content that will be used as part of the style attribute as trusted.

Concatenating malicious user content into the style attribute can result in clickjacking attacks (like this one that affected LinkedIn in 2015).

This mitigation is less effective than the mitigation against XSS because:

• It does not offer a pattern for escaping user content before marking a string as trusted, which leads many developers to mark strings as trusted to "make the error go away".
• It is a defense-in-depth strategy rather than a complete mitigation.

We may want to reconsider this mitigation, but it would be inappropriate (in my opinion) to weaken a Defense in Depth strategy simply because we're strengthening an unrelated mitigation.

Trusted Types

Trusted Types offer a way to address XSS (but not exfiltration or clickjacking).

Before Trusted Types, it was possible to eliminate sources of XSS vulnerabilities through CSP opt-ins. These opt-ins work in a blunt-force way: they disable all of the DOM "sinks" that can result in the execution of dynamic code.

Trusted Types build on this foundation by allowing applications to mark specific dynamic strings as trusted, allowing them to be passed to DOM sinks that execute code even when stricter CSP settings are enabled.

(The API is carefully crafted to require the application author (the person who is responsible for the HTML code that bootstrapped the app) to have a direct hand in marking a particular string as trusted.)

Ultimately, this should result in wider adoption of stricter CSP settings in situations where a small amount of dynamic content (e.g. localization bundled fetched as JSON) currently prevent adoption.

How Ember Should Adopt Trusted Types

Ember's XSS mitigations behave differently from Trusted Types: they work even in the absence of strict CSP settings. In a sense, Ember's templating syntax emulates strict CSP settings, but only when constructing and inserting DOM through Ember templates.

First, a quick comparison of the features.

feature	With CSP	Without CSP	Style Mitigation
Trusted Types	high-fidelity	inactive	out of scope
htmlSafe	imperfect	still active	limited, defense in depth

In my opinion, we want the following changes to Glimmer:

1. When a piece of Glimmer syntax corresponds to a Trusted Types sink (e.g. {{{ corresponds to insertAdjacentHTML), Glimmer should accept the appropriate Trusted Type in addition to a SafeString. This means that the attributes specified as requiring TrustedScriptURL should accept TrustedScriptURL (but not TrustedHTML!) in addition to SafeString.
2. When a string is marked htmlSafe, Glimmer should not turn it into a Trusted Type. This is both because Trusted Types are more granular than htmlSafe and because the current usage of htmlSafe is not well-defined enough to satisfy the security assumptions of a security team that wants to enable strict CSP rules but allow Trusted Types.
3. When Glimmer syntax accepts a SafeString in situations that are out of scope for Trusted Types, we should not accept TrustedHTML or other Trusted Types. This is because marking a string as Trusted HTML explicitly does not imply that it should be trusted in other contexts.

Glimmer Syntax	Accepts htmlSafe?	Accepts Trusted Type?
Corresponds to a Trusted Type sink	Yes, with CSP Disabled1	Yes
Does not Correspond to a Trusted Type Sink	Yes	No

Footnotes

1. When CSP is enabled, any Glimmer syntax that corresponds to a Trusted Types sink will produce an error unless the string is marked as a Trusted Type. This is what we want. ↩

[crypto]

@chancancode is anything else required for this PR?

[chancancode]

…yes?

[chancancode]

I believe the previous comments/reviews still reflect the status quo, has anything changed, or am I missing/misremembering something? Do you want to summarize what you see as the current status and how it aligns with the direction we seem to all wait, and maybe we can find the disconnect that way?