ionic-team / stencil

A toolchain for building scalable, enterprise-ready component systems on top of TypeScript and Web Component standards. Stencil components can be distributed natively to React, Angular, Vue, and traditional web developers from a single, framework-agnostic codebase.
https://stenciljs.com
Other
12.49k stars 783 forks source link

bug: TypeError on Setting 'innerHTML': 'TrustedHTML' Assignment Required #5206

Closed TheodoreGC closed 6 months ago

TheodoreGC commented 8 months ago

Stencil version:

 @stencil/core@4.9.0

Current behavior: When attempting to inject a web component, built using Stencil, into a webpage via a browser extension's content script, an error is encountered:

TypeError: Failed to set the 'innerHTML' property on 'Element': This document requires 'TrustedHTML' assignment.

Expected behavior: The process of injecting a Stencil-generated web component into a webpage through a browser extension's content script should occur seamlessly, without triggering any errors related to content security policies or HTML trust requirements.

Other information: This issue arises due to Stencil's bootstrap process for components, which relies on the innerHTML method. This method can be considered unsafe in the context of Chrome extensions, leading to stricter security checks and the resulting error.

https://github.com/ionic-team/stencil/blob/a69b4d619ebf4f65f54e7ad35c24b28f2416d9c8/src/runtime/bootstrap-lazy.ts#L197 https://github.com/ionic-team/stencil/blob/a69b4d619ebf4f65f54e7ad35c24b28f2416d9c8/src/runtime/bootstrap-lazy.ts#L202

The error encountered indicates a conflict with the Content Security Policy (CSP) typically enforced in browser extensions. CSP aims to mitigate risks associated with cross-site scripting (XSS) and data injection attacks, which are particularly sensitive in the context of extensions that have elevated access to the user's browser.

As a proposed solution, replacing innerHTML with textContent in these contexts could mitigate the issue. textContent is generally considered a safer alternative and aligns well with the security requirements of browser extensions. An example substitution could be as follows:

// Add styles for `slot-fb` elements if any of our components are using slots outside the Shadow DOM
  if (hasSlotRelocation) {
    dataStyles.textContent += SLOT_FB_CSS;
  }

  // Add hydration styles
  if (BUILD.invisiblePrehydration && (BUILD.hydratedClass || BUILD.hydratedAttribute)) {
    dataStyles.textContent += cmpTags + HYDRATED_CSS;
  }

Rationale for prefering textContent over innerHTML here:

In the specific scenario of appending CSS to <style> elements within Stencil's component initialisation process, the adoption of textContent is not only a security enhancement but also aligns with best practices for performance and code clarity. This change would positively impact both browser extensions and traditional web applications, without any loss of functionality or capability.

System Info:

System: node 18.17.1
 Platform: darwin (23.1.0)
CPU Model: Apple M1 (8 cpus)
Compiler: /Users/theo/.nvm/versions/node/v18.17.1/lib/node_modules/@stencil/core/compiler/stencil.js
Build: 1702922611
Stencil: 4.9.0 🐏
TypeScript: 5.2.2
Rollup: 2.42.3
Parse5: 7.1.2
jQuery: 4.0.0-pre
Terser: 5.26.0

Code Reproduction URL:

Stencil Bug Reproduction Repository

Steps to Reproduce:

The steps to reproduce the issue can be found here

ionitron-bot[bot] commented 8 months ago

Thanks for the issue! This issue has been labeled as holiday triage. With the winter holidays quickly approaching, much of the Stencil Team will soon be taking time off. During this time, issue triaging and PR review will be delayed until the team begins to return. After this period, we will work to ensure that all new issues & PRs are properly triaged.

In the meantime, please read our 2023 Winter Holiday Triage Guide for information on how to ensure that your issue is triaged correctly.

Thank you!

rwaskiewicz commented 8 months ago

Hey @TheodoreGC 👋

Thanks for the report! It looks like this was created using an older version of our bug report template. Can you please add the following information to the issue summary for me? This helps us triage/understand issues much more efficiently.

  1. System Info - Output of npx stencil info, browser (e.g. does this only apply to Chrome?), versions of affected browser, and any other environment information you think is important here
  2. Code reproduction URL - a link to a minimal reproduction case for the team to look at
  3. Steps to reproduce - the steps to reproduce this issue with the code reproduction case

Thanks!

ionitron-bot[bot] commented 8 months ago

Thanks for the issue! This issue has been labeled as needs reproduction. This label is added to issues that need a code reproduction.

Please reproduce this issue in an Stencil starter component library and provide a way for us to access it (GitHub repo, StackBlitz, etc). Without a reliable code reproduction, it is unlikely we will be able to resolve the issue, leading to it being closed.

If you have already provided a code snippet and are seeing this message, it is likely that the code snippet was not enough for our team to reproduce the issue.

For a guide on how to create a good reproduction, see our Contributing Guide.

TheodoreGC commented 8 months ago

Hey @rwaskiewicz 👋,

Thank you for the guidance. I've updated the bug report with the additional information you requested. Here's a quick summary:

Thanks again for your assistance!

rwaskiewicz commented 8 months ago

Hey @TheodoreGC - can you please provide some additional information for us?

  1. Can you include the original Stencil source code in your reproduction case? It appears that only the compiled code is in that GitHub repo.
  2. Can you provide the browser(s) and version(s) of those browsers this is occurring in?

Thanks!

TheodoreGC commented 8 months ago

Hey @rwaskiewicz,

Thanks for reaching out. I've updated the GitHub repository to include the full project of the web component. This should give a clearer picture.

Additionally, I've revised the README to provide detailed instructions on how to install the component. This will help in setting up the environment for testing and reproducing the issue.

Regarding the browsers, I've tested this behaviour on the following:

I hope this information helps. Let me know if you need anything else.

Thanks!

rwaskiewicz commented 8 months ago

Thanks! I've validated that this is a bug, and I've ingested this into our backlog. I'll see if we can get the PR reviewed soon - there's a couple mixed usages of innerHTML and textContent that I want us (the team) to verify/validate everything works as expected. Thanks again!

alicewriteswrongs commented 6 months ago

Hey @TheodoreGC, just set your PR to merge, so it will be included in the next release of Stencil. We'll be sure to ping here when that gets released!

christian-bromann commented 6 months ago

A fix for this has been published in v4.12.3.