Allow for limiting the sinks that a type can be used for

[briansmith]

The draft spec currently has this example:

const cdnScriptsPolicy = TrustedTypes.createPolicy('cdn-scripts', {
  createScriptURL(url) {
    const parsed = new URL(url, document.baseURI);
    if (parsed.origin == 'https://mycdn.example') {
      return URL.toString();
    }
    throw new TypeError('invalid URL');
  },
});

First, is there a typo in this example? I am assuming it was intended to have return parsed.toString(); instead of return URL.toString();.

More to the point, if I understand the example correctly, this policy will replace any/all relative URI references and some non-normalized URI references with a different URI reference that is absolute and somewhat normalized. Also, it will (I think) reject some malformed strings. Questions:

1. Is the rejection of malformed strings here intentional? It would be good to document that users of TrustedTypes should generally some, but not all, malformed URI references to be rejected.

2. Is the conversion from a relative URI reference to absolute URI reference intetional? In particular, is the example trying to defend against a TOCTOU issue where document.baseURI changes between the time the TrustedScriptURL is constructed and the time it is used in the document? Are there other reasons one should do the relative-to-absolute conversion? I think any/all of these reasons we can come up with should be called out.

3. In particular, the TOCTOU issue regarding document.baseURI and mitigations for it (e.g. using CSP base-uri) should be called out. Further, I think it would be useful to consider creating a special TrustedBaseURL and associated infrastructure (createBaseURL, etc.) for HTMLBaseElement.href to mitigate this, as most TrustedTypes users probably don't document.baseURI to change dynamically at all. I expect most implementations of createURL will fail to consider the possibility that the URL will be used for a <base> element.

4. Since the policy may transform the input URI reference to another URI reference, users of trusted types need to either lower their expectations about the form of the URI references in their document, or they need to make sure they use a policy that was designed to accommodate their expectations. For example, an application that assumes that all links in the document are relative URI references would need to be modified to stop making that assumption, or it would need to use a policy that accommodates this assumption. For example, here is a slight modification of the example policy that accommodates applications that make strong assumptions about the URI references they create:

const cdnScriptsPolicy = TrustedTypes.createPolicy('cdn-scripts', {
  createScriptURL(url) {
    const parsed = new URL(url, document.baseURI);
    if (parsed.origin == 'https://mycdn.example') {
      return url; // <----- return the input as the output.
    }
    throw new TypeError('invalid URL');
  },
});

[koto]

The draft spec currently has this example:

const cdnScriptsPolicy = TrustedTypes.createPolicy('cdn-scripts', {
  createScriptURL(url) {
    const parsed = new URL(url, document.baseURI);
    if (parsed.origin == 'https://mycdn.example') {
      return URL.toString();
    }
    throw new TypeError('invalid URL');
  },
});

First, is there a typo in this example? I am assuming it was intended to have return parsed.toString(); instead of return URL.toString();.

No, this one is expected. In this particular policy we canonicalize the URL.

More to the point, if I understand the example correctly, this policy will replace any/all relative URI references and some non-normalized URI references with a different URI reference that is absolute and somewhat normalized. Also, it will (I think) reject some malformed strings.

Correct.

Questions:

1. Is the rejection of malformed strings here intentional? It would be good to document that users of TrustedTypes should generally [cause] some, but not all, malformed URI references to be rejected.

Yes. We're leaving the implementation of the policies to the users (we are following up with somwe reference implementation & documentation of policies)

2. Is the conversion from a relative URI reference to absolute URI reference intetional? In particular, is the example trying to defend against a TOCTOU issue where document.baseURI changes between the time the TrustedScriptURL is constructed and the time it is used in the document? Are there other reasons one should do the relative-to-absolute conversion? I think any/all of these reasons we can come up with should be called out.

In this policy it's intentional, TOCTOU issue with the base URI is an issue, but rather this is done for clarity. It might be impractical to use absolute URLs everywhere in the document, but in that case the base URI TOCTOU is a security concern indeed, we should put it in an informational note or security considerations.

3. In particular, the TOCTOU issue regarding document.baseURI and mitigations for it (e.g. using CSP base-uri) should be called out. Further, I think it would be useful to consider creating a special TrustedBaseURL and associated infrastructure (createBaseURL, etc.) for HTMLBaseElement.href to mitigate this, as most TrustedTypes users probably don't document.baseURI to change dynamically at all. I expect most implementations of createURL will fail to consider the possibility that the URL will be used for a <base> element.

Thoughts, @otherdaniel @mikesamuel @xtofian? I personally would rather refrain from adding types for single sinks. Perhaps base uri can be mapped to TrustedScriptURL for example (as the worst consequence is indeed loading a script).

4. Since the policy may transform the input URI reference to another URI reference, users of trusted types need to either lower their expectations about the form of the URI references in their document, or they need to make sure they use a policy that was designed to accommodate their expectations. For example, an application that assumes that all links in the document are relative URI references would need to be modified to stop making that assumption, or it would need to use a policy that accommodates this assumption. For example, here is a slight modification of the example policy that accommodates applications that make strong assumptions about the URI references they create:

const cdnScriptsPolicy = TrustedTypes.createPolicy('cdn-scripts', {
  createScriptURL(url) {
    const parsed = new URL(url, document.baseURI);
    if (parsed.origin == 'https://mycdn.example') {
      return url; // <----- return the input as the output.
    }
    throw new TypeError('invalid URL');
  },
});

Yes (with the caveat that this is for script URLs, not URLs in general). We also discuss this in #151. This is the case in general - policies may transform their input, so write the policies in the way that doesn't break your application (unless if it only "breaks" unintentional behaviour like DOM XSS).

[briansmith]

First, is there a typo in this example? I am assuming it was intended to have return parsed.toString(); instead of return URL.toString();.

No, this one is expected. In this particular policy we canonicalize the URL.

Hmm...I don't understand what URL.toString() is supposed to do here. If I evaluate it in a console window I get:

> URL.toString()
"function URL() { [native code] }"

[koto]

First, is there a typo in this example? I am assuming it was intended to have return parsed.toString(); instead of return URL.toString();.

No, this one is expected. In this particular policy we canonicalize the URL.

Hmm...I don't understand what URL.toString() is supposed to do here. If I evaluate it in a console window I get:

> URL.toString()
"function URL() { [native code] }"

Ah, I see the issue - that was a clear bug, I assumed the policy code returns the serialized parsed URL, fixed it in #153.

[koto]

Brian, does the commit address the issue? Would the appropriate resolution be to add a note to the spec that the relative / absolute URL mismatch may introduce a vulnerability via changing the base URI?

The alternates are:

• making URLs always absolute (I'd rather avoid it, it's not backwards compatible)
• introducing TrustedBaseURL (which technically still allows for vulnerable code)
• freezing base URL

[briansmith]

I think people will feel more comfortable adopting Trusted-Types if in createURL and createScriptURL the input URL is equal to the output URL for any allowed URL. That is, I think it would be good to change things so that people normally write the second return url; form of policy I mentioned at the end of #152 (comment), instead of the return parsed.toString(); form.

I expect that most people who would use Trusted Types don't want <base> to be used at all and so it should be easy to implement the equivalent of CSP base-uri 'none' (and maybe base-uri 'self') in a Trusted-Types policy. This makes me think that base URLs shouldn't have the same type as other URLs.

I also think that Trusted Types works more like insecure blacklist-style sanitation. Inevitably, the number of sinks will increase over time. When you implement createURL and createScriptURL today you have to consider all possible sinks, but that's impossible because you can't predict the future. Maybe it would make more sense for TrustedURL and TrustedScriptURL to indicate which sinks were considered by the policy. So, for example, today it is claimed there are ~50 URL sinks, so my library would have a set of those 50 sinks called urlSinks and then the policy would return something like { url: url, sinks: urlSinks }. Then the browser could block the use of the URL for any sink that isn't explicitly mentioned. This would transform the mechanism into more of a whitelist-based style.

Similarly, the way the web APIs are annotated for Trusted Types seems too blacklist-y to me. Ideally, every web feature would be annotated with an indication of its Trusted Type, and any API not explicitly annotated would be assumed to be an XSS sink, for which no trusted type could be used, until such an annotation is added.

[koto]

I think people will feel more comfortable adopting Trusted-Types if in createURL and createScriptURL the input URL is equal to the output URL for any allowed URL. That is, I think it would be good to change things so that people normally write the second return url; form of policy I mentioned at the end of #152 (comment), instead of the return parsed.toString(); form.

Fair point, changed in 29ca732. Thanks!

I expect that most people who would use Trusted Types don't want <base> to be used at all and so it should be easy to implement the equivalent of CSP base-uri 'none' (and maybe base-uri 'self') in a Trusted-Types policy. This makes me think that base URLs shouldn't have the same type as other URLs.

Extracted to #172.

I also think that Trusted Types works more like insecure blacklist-style sanitation. Inevitably, the number of sinks will increase over time. When you implement createURL and createScriptURL today you have to consider all possible sinks, but that's impossible because you can't predict the future. Maybe it would make more sense for TrustedURL and TrustedScriptURL to indicate which sinks were considered by the policy. So, for example, today it is claimed there are ~50 URL sinks, so my library would have a set of those 50 sinks called urlSinks and then the policy would return something like { url: url, sinks: urlSinks }. Then the browser could block the use of the URL for any sink that isn't explicitly mentioned. This would transform the mechanism into more of a whitelist-based style.

We plan to provide an authoritative information about the sinks that a given type will cover for a current document via #149, so the policy could reject creating a type if the mapping is too 'lax' for it. But what you propose is quite different indeed. Effectively, typed objects start being parametrized (each instance would become TrustedURL<AllowedSinks>) and not interchangeable, making reasoning about the program and debugging more tricky. Depending on the implementation (e.g. if policy dynamically decides the sink list like above), it would be impossible to infer statically if the DOM will not reject a variable (TT in general allow us to make such checks, with some caveats around eval).

Authors can implement additional sink limitations with Trusted Types, e.g. by creating their own types wrapping TT and requiring them for some sinks (via sink setters replacement, or by banning the direct use of some setters, and requiring a specific function call), but I think there's value in making any instance of a given Trusted Type interchangeable (i.e. adhere to the same contract), as that's what's one of the core benefits of a type system.

Similarly, the way the web APIs are annotated for Trusted Types seems too blacklist-y to me. Ideally, every web feature would be annotated with an indication of its Trusted Type, and any API not explicitly annotated would be assumed to be an XSS sink, for which no trusted type could be used, until such an annotation is added.

I agree this would be ideal, but I don't think any user agent would be willing to implement a strongly typed version of each of the already existing, and all future web APIs. Just like script-src has to keep up with the new script-y behavior, same goes for Trusted Types. We're trying to make that more tractable via appropriate spec lingo, such that the TT changes are easy to explain in other specs - e.g. #168.

That said, I feel confident that if the scope of the typed approach remains narrowly DOM XSS, it's possible for the spec bodies to keep up with the new script-related behavior of the web (just like for CSP).

Correctly managing the mapping of types feels like it should be done in a spec, even if it starts as a blacklist; otherwise we'd have to rely on the authors to come up with the correct whitelist on their own (and there might some some interoperability problems down the road).

[briansmith]

Depending on the implementation (e.g. if policy dynamically decides the sink list like above), it would be impossible to infer statically if the DOM will not reject a variable (TT in general allow us to make such checks, with some caveats around eval).

This could be dealt with by having the policy registration pass in the sinks for which the policy is aware, instead of having createURL. This way it would be static per policy.

[koto]

That's true, however if multiple sinks are allowed by a policy, it might be practically hard to express that in a type language (I'm not sure this is actually the case here, calling @engelsdamien for TypeScript as an example - createElement from lib.dom.d.ts has something similar, but only for a single parameter, not a list of them).

I'm also worried that it adds new API surface (e.g. TrustedURL.getAllowedSinks) and makes the manual debugging harder.

I understand the risk of new sinks coming up, but I feel this should be dealt with at a spec level, by spec editors, and we should not add more work for the authors (in this case - to specify a whitelist of sinks), that has the side-effects that affects them. One idea on how to make this future-proof is #176.

[engelsdamien]

It is a little hard to tell how difficult it would be to express in typescript without first seeing what the api would look like.

One thing in particular I did not understand in the proposal is how would the sinks be expressed, that is, what does an element of the urlSinks set look like ?

That being said, I suspect this would end up being expressed as TrustedURL<AllowedSinks> where AllowedSinks is encoded as a union of all the sink types. Even though this is possible to do, the resulting type would probably be huge and lead to a poor developper experience.

[koto]

If I understand correctly, the use case would be for the authors to define a stricter rule set than what the current (post-TT) model is. For example limiting what can create a type for a given sink (say, iframe.src or base.href). Whitelist of sinks per-policy (statically, or dynamically defined) might be unpractical to achieve that goal, because every policy would have to specify rules that do not violate this restriction - so for example, the authors might need to verify that every policy but the chosen ones cannot produce a type valid for base.href. This might have big interoperability costs, as most likely now you not only have to review a policy, you need to be able to author it (to modify its rules to adhere to a custom enforcement). I imagine it would be a difficult task for libraries to offer a support for this. For example, React library would probably cause a lot of runtime errors if faced with DOM API sinks that behave differently in every application (how would the error recovery look like?), whereas it's relatively easy to support baseline TT + raw DOM variants only.

By agreeing on some baseline enforcement on the DOM sinks (a collection of type => sinks relations) we are able to enforce some rules while staying interoperable. That is something that can be specced, implemented and maintained, but more importantly - used with little functional breakage risk.

It sounds like the custom logic could be added in user code already, and a better place would be sinks, since it's them that trigger the enforcement. For example, like this: https://jsbin.com/gefoxaquda/edit

[koto]

Closing for the reasons mentioned earlier. We may revisit this for the next iterations of the API, but it seems like it increases the complexity of the design with little gain :/