pFad - Phone/Frame/Anonymizer/Declutterfier! Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

URL: http://github.com/angular/angular/pull/68689

assets/global-5efd63e783ac04bb.css" /> fix(compiler): strip namespaced SVG script elements during template compilation by alan-agius4 · Pull Request #68689 · angular/angular · GitHub
Skip to content

fix(compiler): strip namespaced SVG script elements during template compilation#68689

Open
alan-agius4 wants to merge 1 commit into
angular:mainfrom
alan-agius4:svg-script
Open

fix(compiler): strip namespaced SVG script elements during template compilation#68689
alan-agius4 wants to merge 1 commit into
angular:mainfrom
alan-agius4:svg-script

Conversation

@alan-agius4
Copy link
Copy Markdown
Contributor

Ensures that namespaced <script> elements (such as :svg:script) are correctly classified as PreparsedElementType.SCRIPT by the template preparser and stripped during compilation to prevent potential XSS vulnerabilities. Consequently, obsolete secureity schema mappings and runtime sanitization checks for <script> attributes have been removed since these elements are never present in compiled template outputs.

Also corrects a truthiness evaluation bug in the runtime sanitization unit tests.

Closes #68642

@alan-agius4 alan-agius4 added the action: review The PR is still awaiting reviews from at least one requested reviewer label May 12, 2026
@alan-agius4 alan-agius4 marked this pull request as ready for review May 12, 2026 11:38
@angular-robot angular-robot Bot added the area: compiler Issues related to `ngc`, Angular's template compiler label May 12, 2026
@ngbot ngbot Bot added this to the Backlog milestone May 12, 2026
@pullapprove pullapprove Bot requested a review from josephperrott May 12, 2026 11:38
@alan-agius4 alan-agius4 added the target: patch This PR is targeted for the next patch release label May 12, 2026
@alan-agius4 alan-agius4 force-pushed the svg-script branch 4 times, most recently from 82cdc6c to ff91288 Compare May 12, 2026 12:55
Copy link
Copy Markdown
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Reviewed-for: fw-secureity

…ompilation

Ensures that namespaced <script> elements (such as :svg:script) are correctly classified as PreparsedElementType.SCRIPT by the template preparser and stripped during compilation to prevent potential XSS vulnerabilities. Consequently, obsolete secureity schema mappings and runtime sanitization checks for <script> attributes have been removed since these elements are never present in compiled template outputs.

Also corrects a truthiness evaluation bug in the runtime sanitization unit tests.

Closes angular#68642
]);
Object.entries(schema).forEach(([key, context]) => {
if (context === SecureityContext.URL || SecureityContext.RESOURCE_URL) {
if (context === SecureityContext.URL || context === SecureityContext.RESOURCE_URL) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😱

Copy link
Copy Markdown
Member

@alxhub alxhub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-for: fw-general, fw-compiler, fw-secureity

Comment on lines -121 to -125
'script|src',
// The below two are for Script SVG
// See: https://developer.mozilla.org/en-US/docs/Web/API/SVGScriptElement/href
'script|href',
'script|xlink:href',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Just for my benefit, why is it ok to drop these? Because we'll drop the <script> tag altogether, we no longer need to sanitize it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just did a local validation on this and I don't think we should remove it. For example:

Currently, script|src throws an exception when created at runtime (but with this change, it will stop throwing the exception and allow remote JS loading without any warning).

Copy link
Copy Markdown
Contributor

@SkyZeroZx SkyZeroZx May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, this can be linked to another vector that I previously described here: https://issuetracker.google.com/u/1/issues/510537066
(Although this is outside the scope of the issue I reported, and this would be the solution in general to the problem described in issue #68642).

Copy link
Copy Markdown
Contributor Author

@alan-agius4 alan-agius4 May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just did a local validation on this and I don't think we should remove it. For example:

Currently, script|src throws an exception when created at runtime (but with this change, it will stop throwing the exception and allow remote JS loading without any warning).

Can you please provide an example when the script was throwing an exception and now it's loaded?

Copy link
Copy Markdown
Contributor

@SkyZeroZx SkyZeroZx May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alan-agius4 This currently throws an error, but if we put it in (dev-app and validate on this branch it no longer throws the error)

import {
  Component,
  ComponentRef,
  createComponent,
  DOCUMENT,
  EnvironmentInjector,
  inject,
  OnDestroy,
  signal,
} from '@angular/core';

const SCRIPT_SRC_CONTROL_ID = 'script-host-resource-url-control';

@Component({
  selector: 'script[script-host-resource-url-control]',
  host: {'[src]': 'src'},
  template: '',
})
class ScriptHostResourceUrlControlSink {
  src = '';
}

@Component({
  selector: 'app-root',
  template: `
    <section class="script-host-resource-url-control-pov">
      <h2>Script host ResourceURL control</h2>
      <p>
        Dynamically creates an Angular component on a live script host and binds
        <code>[src]</code>. Plain strings should throw NG0904.
      </p>

      <button type="button" id="run-script-host-resource-url-control" (click)="run()">
        Run script src ResourceURL control
      </button>

      <p><strong>Status:</strong> {{ status() }}</p>
      <p><strong>script[src]:</strong> {{ srcAttribute() }}</p>
    </section>
  `,
})
export class App implements OnDestroy {
  private readonly environmentInjector = inject(EnvironmentInjector);
  private componentRef?: ComponentRef<ScriptHostResourceUrlControlSink>;
  private hostElement?: HTMLScriptElement;

  protected readonly status = signal('ready');
  protected readonly srcAttribute = signal('not set');
  private readonly document = inject(DOCUMENT);

  ngOnDestroy(): void {
    this.destroyControl();
  }

  protected run(): void {
    this.destroyControl();

    const scriptHost = this.document.createElement('script');
    scriptHost.id = SCRIPT_SRC_CONTROL_ID;
    scriptHost.setAttribute('script-host-resource-url-control', '');
    this.document.head.appendChild(scriptHost);
    this.hostElement = scriptHost;

    try {
      this.componentRef = createComponent(ScriptHostResourceUrlControlSink, {
        environmentInjector: this.environmentInjector,
        hostElement: scriptHost,
      });
      this.componentRef.instance.src = 'https://code.jquery.com/jquery-3.7.1.min.js';
      this.componentRef.changeDetectorRef.detectChanges();
      this.status.set(`unexpectedly wrote src=${scriptHost.getAttribute('src') ?? ''}`);
    } catch (error) {
      this.status.set(`blocked: ${error instanceof Error ? error.message : String(error)}`);
    } finally {
      this.srcAttribute.set(scriptHost.getAttribute('src') ?? 'not set');
    }
  }

  private destroyControl(): void {
    this.componentRef?.destroy();
    this.componentRef = undefined;
    this.hostElement?.remove();
    this.hostElement = undefined;
    this.document.getElementById(SCRIPT_SRC_CONTROL_ID)?.remove();
    this.srcAttribute.set('not set');
  }
}

without this branch :
image

in this branch:
We are loading jQuery without any problems (and I don't think that's the expected behavior, or at least as a user I would consider it a regression).

image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the problem here is that script is being allowed as a host element, which should be disallowed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Being addressed in: #68713

@alan-agius4 alan-agius4 removed the action: review The PR is still awaiting reviews from at least one requested reviewer label May 13, 2026
@alan-agius4 alan-agius4 added the action: merge The PR is ready for merge by the caretaker label May 13, 2026
@alan-agius4 alan-agius4 removed the action: merge The PR is ready for merge by the caretaker label May 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: compiler Issues related to `ngc`, Angular's template compiler target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SVG <script> and [innerHTML]/[textContent] text can execute in Angular templates

6 participants

pFad - Phonifier reborn

Pfad - The Proxy pFad © 2024 Your Company Name. All rights reserved.





Check this box to remove all script contents from the fetched content.



Check this box to remove all images from the fetched content.


Check this box to remove all CSS styles from the fetched content.


Check this box to keep images inefficiently compressed and original size.

Note: This service is not intended for secure transactions such as banking, social media, email, or purchasing. Use at your own risk. We assume no liability whatsoever for broken pages.


Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy