Type checking with security sanitization in place

[rictic]

When running in an environment with https://github.com/WICG/trusted-types* the types of some security sensitive built-in attributes and properties are effectively changed. For example, an iframe's src isn't string, but TrustedUrl.

Full list of overrides here: https://wicg.github.io/trusted-types/dist/spec/#enforcement-in-sinks

As the spec isn't yet finalized, the types aren't yet included automatically in the main typings. Further, inside google we use a slightly different system for the time being based on closure library types (e.g. https://google.github.io/closure-library/api/goog.html.SafeUrl.html).

Type typing these with these types doesn't have to be perfect, the canonical security layer is at runtime.

*or when running lit-html with lit/lit#750 enabled, which is kinda-sorta a ponyfill of the trusted types spec purely for lit-html templates

[rictic]

I've got something really hacky working by just checking the names of the types, rather than needing to have a reference to the proper types. The core of the check is roughly:

const securityOverrideMap = {
  'iframe': {
    'src': ['TrustedResourceUrl'],
  },
  'a': {
    'href': ['TrustedResourceUrl', 'SafeUrl', 'string'],
  },
  'img': {
    'src': ['TrustedResourceUrl', 'SafeUrl', 'string'],
  },
  // ... etc
};
const globalSecurityOverrides = {
  'style': ['SafeStyle', 'string'],
};

function checkSecurityAssignability(typeB, htmlAttr, document) {
  const overrideNode = securityOverrideMap[htmlAttr.htmlNode.tagName];
  const overriddenTypes = (overrideNode && overrideNode[htmlAttr.name]) || (globalSecurityOverrides[htmlAttr.name]);
  if (!overriddenTypes) {
    return undefined;
  }
  if (!matchOverriddenTypes(overriddenTypes, typeB)) {
    return [{
      kind: exports.LitHtmlDiagnosticKind.INVALID_ATTRIBUTE_EXPRESSION_TYPE,
      message:
          `Type '${tsSimpleType.toTypeString(typeB)}' is not assignable to '${
              overriddenTypes.join(' | ')}'.`,
      severity: 'error',
      location: {document, ...htmlAttr.location.name},
      htmlAttr,
      typeB
    }];
  }
  return [];
}

function matchOverriddenTypes(typeNames, typeB) {
  if (typeNames.includes(typeB.name)) {
    return true;
  }
  switch (typeB.kind) {
    case 'UNION':
      return typeB.types.every((t) => matchOverriddenTypes(typeNames, t));
    case 'STRING_LITERAL':
    case 'STRING':
      return typeNames.includes('string');
    default:
      return false;
  }
}

[runem]

I would like to start making progress on this issue now. Before I start, I have some thoughts/questions that I want to clarify:

1. I would like to enable trusted type support through a config option: trustedTypes: true (or safeTypes: true). Alternatively, if trusted types are always present, I fear it might be confusing for users to see that various types are a union of eg. string | SafeUrl.

2. When trusted types are enabled, should a given trusted type be the only type available where applicable or should it also be possible to use existing types? Eg. string | SafeUrl vs just SafeUrl.

3. Do you think it would make sense to extend with types from both WICG/trusted-types and closure safe types at the same time? (eg. a.href: TrustedResourceUrl | SafeUrl | string | TrustedURL). Or do you think I should just focus on closure safe types for now?

4. I think I will end up type checking type names like your prototype. This seems to be the most simple and reliable way as of now.

[rictic]

Sweet! Fair warning, things are a little up in the air still with trusted types, so depending on how much we want to keep iterating on this it might make sense to wait for them to stabilize a bit more before landing this.

1. Seems like a good idea to at least start with it behind a flag. Trusted Types are promising but they're not an accepted standard yet.

2. This might need to be behind a config as well. As I understand it, a page may or may not specify a default trusted types policy that can accept (and possibly sanitize) strings before they're passed along to DOM sinks. They can do this on a per type basis, so to accurately map this onto people's options, the config should be able to specify whether to allow string for each of the four types (TrustedHTML, TrustedURL, TrustedScriptURL, and TrustedScript).

3. We're working on integrating lit-html with Trusted Types in Add trusted types support to lit html lit/lit#970. It might make sense to first wait for that to land first before landing TrustedTypes support (or even to wait for the spec to stabilize a bit more, it sounds like things are still changing). I'm unsure.

4. +1

One subtle thing is that the security types should only apply to data bindings. So this is fine, because the url is from a programmer-written constant:

html`
  <script src="/foo.js"></script>
`;

But this should be rejected, because there's no way for the runtime to tell that the url doesn't come from untrusted resource:

const url = '/foo.js';
html`
  <script src="${url}"></script>
`;

[rictic]

Related bug: microsoft/TypeScript#30024