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

Support application-specific sanitizers / type builders #32

Closed
koto opened this issue Dec 1, 2017 · 2 comments
Closed

Support application-specific sanitizers / type builders #32

koto opened this issue Dec 1, 2017 · 2 comments

Comments

@koto
Copy link
Member

koto commented Dec 1, 2017

Extracted off #7, by @xtofian:

[...] There isn't a universal definition of "safe" attached to the types as per the spec, but there certainly is a notion of "safe" that developers/reviewers would attach to these types in the context of their specific code base. The definition of "safe" is whatever makes sense in the context of their app, and is embodied in the implementation of builder/producer APIs that create values of TrustedTypes through use of unsafelyCreate.

[...]

One implication that falls out of this is that any factory functions that are defined in the spec (e.g. TrustedURL.sanitize) must be designed such that the values they produce are within any conceivable notion of "safe" for their corresponding sinks, across applications. This is not necessarily trivial. For instance, should TrustedURL.sanitize accept tel: URLs? On one hand, they're certainly benign with respect to XSS. However, there are contexts where they are problematic (or at least used to be -- e.g., within webviews in iOS, see discussion at golang/go#20586 (comment)). And, it's not even clear that the standard should permit arbitrary http/https URLs. For instance, an application might want to ensure that it only renders hyperlinks that refer within the scope of this application itself, and that links to external sites go through a redirector that performs real-time phishing detection or some such.

This indicates that perhaps the spec shouldn't provide for factory methods for these types at all. This might be a quite reasonable direction: I'd expect these types in the currently spec'd form (with very basic factory functions) to not be all that useful on their own.

To be practically useful, these types will need factories/builders for many common scenarios, e.g.:

  • composability of SafeHtml/TrustedHTML (i.e. the property that for all s, t : SafeHtml, s + t is also in SafeHtml), and a corresponding function that concatenates TrustedHTML (see e.g. SafeHtml.create).
  • factory functions that create HTML tags with tricky semantics, accounting for browser-specific quirks (example); of course this won't be an issue in spec-compliant browsers that implement TrustedTypes, but will have to be accounted for in libraries that support other browsers via polyfill of those types.
  • special case functions for safe-URL types, e.g. to create a blob: URL for a Blob, but only if it's of a MIME type that is considered to not result in content-sniffing issues (e.g., SafeUrl.fromBlob).
  • factory functions for TrustedScriptURL that choose a particular tradeoff between strictness and expressiveness (e.g. TrustedResourceUrl.format).
  • etc.

Of course, all of these factory functions can be replaced with ad-hoc code that relies on unsafelyCreate. However, that's very undesirable since it'll result in a relatively large number of such calls throughout the application source code, which will significantly erode the reviewability benefits.

I.e.. I'd expect these types to be used as a core building block of framworks and libraries that endow them with richer semantics (and corresponding builders and factories), such as the Closure SafeHtml types and their builders, or the SafeHtml types in Angular.

(aside: the one factory function in the current draft spec that's reasonably uncontroversial is TrustedHTML.escape -- escaped text should be within the notion of "safe for HTML" in the context of any conceivable application; however it's also not particularly useful in practice -- applications should just assign to textContent instead of using innerHTML with TrustedHTML.escape).

@xtofian
Copy link

xtofian commented Dec 5, 2017

It seems reasonable to argue that the combination of

forms a sufficient, minimal, and non-opinionated approach to supporting application-specific builders.

This approach would have the (arguably desirable) effect that the web platform provides only those aspects that are difficult and/or inefficient to implement as a JS library/framework:

  • complete mediation of assignments to DOM and JS injection sinks
  • natively-implemented types that can more rigorously ensure their contracts

But at the same time it wouldn't stipulate a particular contract for these types. As noted, the effective contract for the types arises from the uses of unsafelyCreate in scope of the application.

If the spec were to go down this route, it would make sense to release a reference library (let's say "trusted-types-builders") of builders that endow the types with semantics that are appropriate in a typical web application; the builders for the Closure SafeHtml types are probably a good starting point.

One desirable aspect of releasing "typical" builders as a library is that it's much easier to evolve/change than the web platform; and of course it doesn't lock applications into a particular semantics if they have the need for special treatment (for example, code used to build a web app served in a web view, or a framework like electron).

An typical web app would depend on trusted-types-builders, possibly with application-specific extensions (e.g., URL sanitizers specific to the application or organization). It would use the mechanism of #31 to constrain calls to unsafelyCreate from outside of these libraries.

This may also help avoid the need for libraries such as jquery to resort to using unsafelyCreate which would be very undesirable: It would be reasonable for, say, jquery to depend on trusted-types-builders, e.g. to compose small snippets of HTML. But, as long as there are no calls to unsafelyCreate in jquery, it'd be outside of the TCB with respect to the risk of XSS vulnerabilities.

@koto
Copy link
Member Author

koto commented Jun 4, 2018

This laid the groundwork for the policy design of the new API. Thanks for your help!

@koto koto closed this as completed Jun 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants