TrustedHTML.unwrap vs TrustedHTML.prototype.toString

[xtofian]

It's unsound to accept a TrustedType as an input to a builder API without performing a runtime check.

E.g.,

class TrustedHTMLs {
  /**
   * @param {TrustedHTML} h1
   * @param {TrustedHTML} h2
   */
  static concat(h1, h2) {
    return TrustedHTML.unsafelyCreate('' + h1 + h2);
  }

is incorrect because there's no guarantee that run-time types of s1 and s2 are indeed TrustedHTML at a given call site (this is the case even if the code is compiled and statically type checked, since Closure as well as TypeScript type systems are unsound).

To address this concern, we made two design choices in the Closure SafeHtml types:

• The type's toString method returns a debug string that includes a type name. This helps prevent code that needs the string-value of an instance of the type from relying on toString().
• There's a static method SafeHtml.unwrap that performs a run-time type check of its argument.

With that, the correct way to write a concat method is,

  static concat(h1, h2) {
    return TrustedHTML.unsafelyCreate(
        TrustedHTML.unwrap(h1) + TrustedHTML.unwrap(h2));
  }

If we were to follow the capability/factory-based approach to controlling access to unsafelyCreate (proposed in #33 (comment)), it may make sense to provide the unwrap functions as part of the factory as well, to allow trusted-types builder code to be more confident that it's calling the genuine unwrap method.

[koto]

Would instanceof checks be trustworthy enough here?

I see that SafeTypes do:

  if (safeHtml instanceof goog.html.SafeHtml &&
      safeHtml.constructor === goog.html.SafeHtml &&
      safeHtml.SAFE_HTML_TYPE_MARKER_GOOG_HTML_SECURITY_PRIVATE_ ===
          goog.html.SafeHtml.TYPE_MARKER_GOOG_HTML_SECURITY_PRIVATE_) {
    return safeHtml.privateDoNotAccessOrElseSafeHtmlWrappedValue_;
  }

What's the reason for the type marker? I believe we can make TT satisfy the instanceof checks and protect the inner value, e.g. with Symbols, but it'd be far to claim this is unforgeable. The threat model does not really include a malicious developer running the code in the execution context (they might be able to fool the type system in many ways, hooking into prototype functions of Function for example).

[xtofian]

You're right, the threat model indeed assumes a non-malicious developer. However, we have found over the years that we do need to consider some notion of an "overly creative developer" that in order to get their work done might chose to write code that they really shouldn't and which (even though the individual piece of code looks correct to them) might weaken or invalidate whole-program correctness/assurance/reviewability properties (such as the ones we're after by using TrustedTypes).

We've found that in practice, we should aim for a level of protection of the security invariants of these APIs where, to invalidate an invariant, a programmer would have to write code that should be quite clearly considered inappropriate by most developers in the org. For example, in Java code, it seems to be in practice reasonable to not have explicit mechanisms to defend against a developer using reflection to poke behind an API surface (and for instance, to implement their own equivalent of unsafelyCreate out of expediency or to avoid security review). For almost all Java developers, using reflection for such purposes is considered sufficiently icky that it tends to not be a problem in practice.

What level of defense against "creative circumvention of API intent" is necessary likely depends on the culture of organizations, and on the language. As a dynamic language without language-native field visibility, JS might require more active defenses. In the Closure types, we went with a horribly verbose field name, and the additional type marker (which TBH I'm not sure how much value it adds); combined with Closure compiler's static enforcement of @private annotations this seems to have large prevented misuse.

I realize that there is only so much we can do. That said, the current implementation of the polyfill seems a bit too easy to get around; I wouldn't be all too surprised to see code like,

/* Make a TrustedHTML because we know what we're doing */
function myMakeTrustedHTML(s) {
  let h = TrustedHTML.escape('');
  h.inner_ = s;
  return h;
}

However, my guess is that we'd be in pretty solid shape if we could get to a point where in order to subvert the intent of the TrustedType system, a developer would have to muck with Function.prototype and similarly central JS machinery -- doing so should feel terribly icky (and risky in terms of stability and functional correctness) to any reasonable developer.

Re: the polyfill: I don't actually think it's necessary to strengthen the field-privacy defenses in the polyfill: As long as an application is tested or deployed to one browser that natively implements TrustedTypes (which presumably won't have the .inner_ field), code that relies on inappropriately futzing with .inner_ won't pass tests.

[koto]

Implemented in the v2 API by using the isHTML and similar functions. These check if the typed object is present in a private WeakMap in addition to making an instanceof check. Creating an object instance is enough to bypass a compiler check, but would fail at runtime. To make that clear, the constructor throws, as it requires a Symbol instance, that is defined in a IIFE.