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

Consider metadata API; building blocks for HTML sanitizers #43

Closed
xtofian opened this issue May 31, 2018 · 7 comments · Fixed by #197
Closed

Consider metadata API; building blocks for HTML sanitizers #43

xtofian opened this issue May 31, 2018 · 7 comments · Fixed by #197
Assignees
Labels
Milestone

Comments

@xtofian
Copy link

xtofian commented May 31, 2018

Browser-side HTML sanitizers are typically implemented by

  • parsing HTML into an inert Document(Fragment)
  • transforming the resulting "untrusted" tree into a safe tree, by pruning or copying based on attribute/element white/black-lists.
  • re-serializing the tree into HTML (not necessary if the "clean" tree can be directly attached to the DOM).

Examples: https://google.github.io/closure-library/api/goog.html.sanitizer.HtmlSanitizer.html, https://github.com/cure53/DOMPurify.

The necessary whitelists are a fairly significant part of the sanitizer's code size (blacklists would be smaller, but blacklists are generally brittle); https://github.com/cure53/DOMPurify/blob/master/src/attrs.js, https://github.com/cure53/DOMPurify/blob/master/src/tags.js, https://github.com/google/closure-library/blob/master/closure/goog/html/sanitizer/attributewhitelist.js, https://github.com/google/closure-library/blob/master/closure/goog/html/sanitizer/tagwhitelist.js

TT introduces implicit knowledge into the DOM API of the security semantics of DOM attributes. It seems worth considering if we can expose this information somehow, so that it can be used to as the basis of a HTML sanitizer's policy, which could then avoid the code size overhead of their own metadata tables.

Caveat: This will likely not obviate the need for sanitizer-specific whitelists/blacklists altogether: There are elements whose attributes don't require sanitization in a typical application's threat model (where the presence of the element in the DOM is application controlled, and only the attribute's value is potentially malicious); while the entire element is undesirable to keep when sanitizing entirely-untrusted HTML markup. E.g., <form> comes to mind.

@koto koto added the spec label Jun 4, 2018
@koto
Copy link
Member

koto commented Oct 16, 2018

This is discussed under WICG/sanitizer-api#1.

@koto
Copy link
Member

koto commented Mar 18, 2019

Apart from minimizing the code of the sanitizers, the metadata might be helpful for other libraries. For example, Angular compiles the template (in AOT mode at build time) that later on are serialized to a series of definitions based on strings. Later on, these strings are serialized into DOM using wrappers over createElement and setAttribute calls here:

https://github.com/angular/angular/blob/master/packages/core/src/view/element.ts#L151

Since in Angular AOT the template itself cannot be influenced by the attacker, this DOM serialization is safe, however to satisfy Trusted Types contract, various TT objects need to be created near the sink there, so this class needs to ship the element,attr->trusted type mapping to be compliant (and keep it in sync with the DOM type requirements for a given browser). cc @otherdaniel @xtofian

@koto
Copy link
Member

koto commented Apr 24, 2019

I noticed that the same behavior affects DOMPurify here.

koto added a commit to koto/trusted-types that referenced this issue Apr 30, 2019
@koto
Copy link
Member

koto commented Apr 30, 2019

I created a first draft of the API in #149 , I'm still not sure about its shape.

This API is useful for migrating existing codebases (to be consumed by DOM builder libraries e.g. in sanitizers to know which type to create when calling setAttribute or setting properties).

The minimal possible shape should be just the getTypeMapping(namespaceURI) function, but the resulting map contains a * entry that should be defaulted to when a given element/tag is not listed (that is how innerHTML property is required, without having to list all possible tag names).

get(Property|Attribute)Type help with resolving the type when an element is known, applying the aforementioned fallback.

I decided to key by the tag names vs the element constructor names, also tag name is uppercased, whereas attribute name is lowercased, but I don't have strong opinions here.

What will follow is the spec change to incorporate that, but please take a look at the draft implementation, so we don't spec & implement an awful API. @otherdaniel @sebmarkbage @mikesamuel.

@xtofian
Copy link
Author

xtofian commented Apr 30, 2019

This generally seems a reasonable API.  One thing that comes to mind is how the 
// TODO(koto): Figure out what to to with <link> (and possible other dependently-typed situations where the type of the attribute depends on the value of another attribute) would influence the shape of the API.

getAttributeType('link', 'href') could return 'ItsComplicated' to indicate that ad-hoc code needs to deal with this situation. This seems like a kludge. Or, getAttributeType could take the specific element as an argument and look at the run-time value of rel to figure out which type is required; but that'd be a very different API.

IOW, it might be time to resolve #6...

@xtofian
Copy link
Author

xtofian commented Apr 30, 2019

Aside: Once getAttributeType is specified, it would be a more precise way to express the "expectedType" table in https://wicg.github.io/trusted-types/dist/spec/#enforce-a-trusted-type-algorithm.

@koto
Copy link
Member

koto commented May 2, 2019

I commented on #6, but I don't think this should be a blocker for the metadata API (we can even return multiple types per attribute if needed). #6 will likely be resolved by changing the type required to the most restricting one, I don't think we should mutate the type based on another attribute value, this is messy.

koto added a commit to koto/trusted-types that referenced this issue May 4, 2019
@koto koto self-assigned this May 4, 2019
koto added a commit to koto/trusted-types that referenced this issue May 7, 2019
koto added a commit to koto/trusted-types that referenced this issue May 8, 2019
Addresses issue w3c#43.

Three new functions are available:
 * TrustedTypes.getAttributeType(tagName, attribute, elementNS, attrNs)
 * TrustedTypes.getPropertyType(tagName, property, elementNS)
 * TrustedTypes.getTypeMapping(NS)

Since the API now has the data about the DOM sinks, this simplifies the TrustedTypeEnforcer.

Added enforcement for non-HTML namespaces.
koto added a commit to koto/trusted-types that referenced this issue May 8, 2019
Addresses issue w3c#43.

Three new functions are available:
 * TrustedTypes.getAttributeType(tagName, attribute, elementNS, attrNs)
 * TrustedTypes.getPropertyType(tagName, property, elementNS)
 * TrustedTypes.getTypeMapping(NS)

Since the API now has the data about the DOM sinks, this simplifies the TrustedTypeEnforcer.

Added enforcement for non-HTML namespaces.
koto added a commit to koto/trusted-types that referenced this issue May 8, 2019
Addresses issue w3c#43.

Three new functions are available:
 * TrustedTypes.getAttributeType(tagName, attribute, elementNS, attrNs)
 * TrustedTypes.getPropertyType(tagName, property, elementNS)
 * TrustedTypes.getTypeMapping(NS)

Since the API now has the data about the DOM sinks, this simplifies the TrustedTypeEnforcer.

Added enforcement for non-HTML namespaces.
koto added a commit that referenced this issue May 8, 2019
Addresses issue #43.

Three new functions are available:

 * TrustedTypes.getAttributeType(tagName, attribute, elementNS, attrNs)
 * TrustedTypes.getPropertyType(tagName, property, elementNS)
 * TrustedTypes.getTypeMapping(NS)

Since the API now has the data about the DOM sinks, this simplifies the TrustedTypeEnforcer.

Added enforcement for non-HTML namespaces.
@koto koto added this to the v1 milestone Jun 24, 2019
@koto koto closed this as completed in #197 Jul 22, 2019
koto added a commit that referenced this issue Jul 22, 2019
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 8, 2019
Implement TrustedTypes.get[PropertyType|AttributeType|TypeMapping].

Bug: 739170, w3c/trusted-types#43
Change-Id: I924a7a62e37dd29456b1e0a9252e45c7210986b0
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 12, 2019
Implement TrustedTypes.get[PropertyType|AttributeType|TypeMapping].

Bug: 739170, w3c/trusted-types#43
Change-Id: I924a7a62e37dd29456b1e0a9252e45c7210986b0
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 13, 2019
Implement TrustedTypes.get[PropertyType|AttributeType|TypeMapping].

Bug: 739170, w3c/trusted-types#43
Change-Id: I924a7a62e37dd29456b1e0a9252e45c7210986b0
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 26, 2019
Implement TrustedTypes.get[PropertyType|AttributeType|TypeMapping].

Bug: 739170, w3c/trusted-types#43
Change-Id: I924a7a62e37dd29456b1e0a9252e45c7210986b0
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 27, 2019
Implement TrustedTypes.get[PropertyType|AttributeType|TypeMapping].

Bug: 739170, w3c/trusted-types#43
Change-Id: I924a7a62e37dd29456b1e0a9252e45c7210986b0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1725641
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Daniel Vogelheim <vogelheim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690669}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 27, 2019
Implement TrustedTypes.get[PropertyType|AttributeType|TypeMapping].

Bug: 739170, w3c/trusted-types#43
Change-Id: I924a7a62e37dd29456b1e0a9252e45c7210986b0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1725641
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Daniel Vogelheim <vogelheim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690669}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Aug 29, 2019
…metadata api., a=testonly

Automatic update from web-platform-tests
[Trusted Types] Implement Trusted Types metadata api.

Implement TrustedTypes.get[PropertyType|AttributeType|TypeMapping].

Bug: 739170, w3c/trusted-types#43
Change-Id: I924a7a62e37dd29456b1e0a9252e45c7210986b0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1725641
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Daniel Vogelheim <vogelheim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690669}

--

wpt-commits: f283c90ff58ce6b758525ba9ef8f7885d466730f
wpt-pr: 18334
xeonchen pushed a commit to xeonchen/gecko that referenced this issue Aug 30, 2019
…metadata api., a=testonly

Automatic update from web-platform-tests
[Trusted Types] Implement Trusted Types metadata api.

Implement TrustedTypes.get[PropertyType|AttributeType|TypeMapping].

Bug: 739170, w3c/trusted-types#43
Change-Id: I924a7a62e37dd29456b1e0a9252e45c7210986b0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1725641
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Daniel Vogelheim <vogelheim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690669}

--

wpt-commits: f283c90ff58ce6b758525ba9ef8f7885d466730f
wpt-pr: 18334
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 4, 2019
…metadata api., a=testonly

Automatic update from web-platform-tests
[Trusted Types] Implement Trusted Types metadata api.

Implement TrustedTypes.get[PropertyType|AttributeType|TypeMapping].

Bug: 739170, w3c/trusted-types#43
Change-Id: I924a7a62e37dd29456b1e0a9252e45c7210986b0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1725641
Reviewed-by: Mike West <mkwstchromium.org>
Commit-Queue: Daniel Vogelheim <vogelheimchromium.org>
Cr-Commit-Position: refs/heads/master{#690669}

--

wpt-commits: f283c90ff58ce6b758525ba9ef8f7885d466730f
wpt-pr: 18334

UltraBlame original commit: 71a9f2456d8feda9827ce92ccbf3c5f7fbc4f0df
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 4, 2019
…metadata api., a=testonly

Automatic update from web-platform-tests
[Trusted Types] Implement Trusted Types metadata api.

Implement TrustedTypes.get[PropertyType|AttributeType|TypeMapping].

Bug: 739170, w3c/trusted-types#43
Change-Id: I924a7a62e37dd29456b1e0a9252e45c7210986b0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1725641
Reviewed-by: Mike West <mkwstchromium.org>
Commit-Queue: Daniel Vogelheim <vogelheimchromium.org>
Cr-Commit-Position: refs/heads/master{#690669}

--

wpt-commits: f283c90ff58ce6b758525ba9ef8f7885d466730f
wpt-pr: 18334

UltraBlame original commit: 71a9f2456d8feda9827ce92ccbf3c5f7fbc4f0df
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 4, 2019
…metadata api., a=testonly

Automatic update from web-platform-tests
[Trusted Types] Implement Trusted Types metadata api.

Implement TrustedTypes.get[PropertyType|AttributeType|TypeMapping].

Bug: 739170, w3c/trusted-types#43
Change-Id: I924a7a62e37dd29456b1e0a9252e45c7210986b0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1725641
Reviewed-by: Mike West <mkwstchromium.org>
Commit-Queue: Daniel Vogelheim <vogelheimchromium.org>
Cr-Commit-Position: refs/heads/master{#690669}

--

wpt-commits: f283c90ff58ce6b758525ba9ef8f7885d466730f
wpt-pr: 18334

UltraBlame original commit: 71a9f2456d8feda9827ce92ccbf3c5f7fbc4f0df
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants