w3c / trusted-types

A browser API to prevent DOM-Based Cross Site Scripting in modern web applications.
https://w3c.github.io/trusted-types/dist/spec/
Other
586 stars 68 forks source link

`execCommand` spec won't work #500

Closed lukewarlow closed 1 month ago

lukewarlow commented 2 months ago

Currently execCommand is specced to just pass the union type value through to createContextualFragment algorithm, but TT enforcement doesn't happen in the algorithm it happens in the IDL. So this won't enforce TT and also won't convert the Trusted type object to a string.

lukewarlow commented 2 months ago

Solution is to do the enforcement before passing the value through to that algorithm.

But pending discussions regarding the [StringContext] IDL attribute, this issue might resolve itself, as createContextualFragment will take a union itself and then do the TT enforcement in the algorithm.

annevk commented 2 months ago

How does this work in Chromium today? Does execCommand() call createContextualFragment() at a time where it might not expect userland to be able to execute code? Does not sound like a great setup.

lukewarlow commented 2 months ago

Chrome like WebKit doesn't actually implement this as specced from what I can see. It does the TT check inside execCommand and then goes through an editor_command file, which doesn't appear to call createContextualFragment anywhere.

annevk commented 2 months ago

It might be better to specify the TT check inside execCommand() then.

https://w3c.github.io/editing/docs/execCommand/ is a bit dramatic about its status. It definitely needs to continue to be maintained.

cc @zcorpan

zcorpan commented 2 months ago

Yes, I've taken on editing of the execCommand spec but haven't gotten to working on it yet.

lukewarlow commented 2 months ago

Okay I've updated https://github.com/w3c/editing/pull/460 to do the enforcement in the editing spec. It will result in the spec doing a double check because createContextualFragment will do one too but the editing spec probably shouldn't call that method anyway, and if it does then we can change the (soon to be upstreamed to HTML) algorithm to take a should do TT check boolean.

annevk commented 2 months ago

It shouldn't call that method directly, ever.