fix(core): reject script element as a dynamic component host#68713
fix(core): reject script element as a dynamic component host#68713alan-agius4 wants to merge 1 commit into
Conversation
To enhance application security and prevent accidental or malicious script execution, this change ensures that dynamically mounting a component via createComponent directly onto a <script> element throws a runtime error in development mode. SVG <script> elements are also rejected. The error message is designed to be fully tree-shakable under production builds where ngDevMode is disabled.
| }) | ||
| class MySvgSink {} | ||
|
|
||
| const svgScriptHost = document.createElementNS('http://www.w3.org/2000/svg', 'script'); |
There was a problem hiding this comment.
I understand this; I would only validate that the host is a script.
We are not validating a case as if it were
<svg>
<script> </script>
</svg>For example
It will not throw an error; it currently has the previously described form (and is also considered validated HTML according to https://developer.mozilla.org/en-US/docs/Web/API/SVGScriptElement)
const svgHost = document.createElementNS(SVG_NS, 'svg');
const text = document.createElementNS(SVG_NS, 'text');
const script = document.createElementNS(SVG_NS, 'script');
svgHost.append(text, script);
this.ref = createComponent(SvgScriptChildPov, {
environmentInjector: this.env,
hostElement: svgHost,
});| encapsulation === ViewEncapsulation.ShadowDom || | ||
| encapsulation === ViewEncapsulation.ExperimentalIsolatedShadowDom; | ||
| const rootElement = renderer.selectRootElement(elementOrSelector, preserveContent); | ||
| if (rootElement.tagName.toLowerCase() === 'script') { |
There was a problem hiding this comment.
Perhaps a simple way would be to prevent any child of the host from having a script (considering we are prohibiting scripts at both the compiler and SVG levels, which would be the overall and retroactive goal if any future HTML spec allows it).
function containsScriptElement(root: Element): boolean {
return root.localName === 'script' || root.querySelector('script') !== null;
}|
@alan-agius4 After investigating and running some more tests, I believe this issue is a bit more complex than simply a I've updated the details here https://issuetracker.google.com/u/1/issues/510537066 with a more complete example. I think this issue should be reclassified. |
To enhance application security and prevent accidental or malicious script execution, this change ensures that dynamically mounting a component via createComponent directly onto a <script> element throws a runtime error in development mode. SVG <script> elements are also rejected. The error message is designed to be fully tree-shakable under production builds where ngDevMode is disabled.
More context in: #68689 (comment)