-
Notifications
You must be signed in to change notification settings - Fork 31
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
Also sanitize javascript: href on MathML and SVG anchor? #168
Comments
See also #147 |
I think we should do that. @otherdaniel thoughts? |
Makes sense, generally. I think the idea was to drop |
This should also happen for all |
@otherdaniel Pls remember to double-check w/ your pull request (e.g., xlink href, ...) |
Part of this might be because there isn't a clearly correct answer :). WebKit and Gecko allow any MathML element to have an href. Chromium allows none of them to have an href. The MathML-Core L1 remains moot on it specifically because it was getting hard to sort out and there were a bunch of issues to discuss. I guess the best entry issue there is w3c/mathml-core#142 -- The Working group would relish the ability to sort it out though so we could get them worked out. L1 is a lot stronger with it, I think. The proposal was perhaps token element and mrow. There was even an argument made iirc that perhaps even just on mrow would be enough. The reason being that tokens can contain foreign content and could therefore at least contain a link pretty easily already, and the math element itself can be wrapped in one - it is really maybe primarily the generic grouping element (mrow) that |
Chromium's model of not supporting |
PR #212 is somewhat preliminary,and is meant to address this issue:
That last bit is rather ad-hoc: It isn't super precide, and forbids some situations that aren't actually dangerous (like an href attribute on an element that doesn't actually support it). It's the simplest check I could think of that leaves the animation support basically intact, but still covers all situations where animtion can be used to create javascript:-URLs. I'm not quite sure what to do with MathML. One option would be to check for any MathML-element with an href attribute. |
Checking |
It also doesn't (iirc) work with related attributes (target, for example) and has unusual default tabdex which threw us way back in whatwg/html#5248 (comment) |
The primary reason why I need (and originally created, for Mozilla Mail/News and Thunderbird) the sanitizer is to remove all Javascript (for security) and all external pings to server (mostly for privacy, partially for security). I think that's what devs will generally expect from a sanitizer, and should be the default. If there's any way whatsoever to run Javascript, the sanitizer is no longer useful. This is particularly important for cases that normal devs do not think about, like MathML and SVG, and they will likely not test that, unless they are security experts. So, I'm in favor of removing href from SVG and MathML. Ditto for any other Javascript and external links. |
We discussed and agreed we should do what @zcorpan said above for MathML. #212 covers what we should do for SVG. The remainder of the concerns should be handled by the safelists of elements and their attributes (e.g., We don't think |
Sounds great to me. I agree it makes sense to preserve links that |
The easiest way of implementing https://wicg.github.io/sanitizer-api/#handle-funky-elements in Gecko will also automatically sanitize the
href
attribute from svg:a and MathML elements. Maybe something to consider.The text was updated successfully, but these errors were encountered: