Skip to content

Commit 2e3d037

Browse files
committed
fix(compiler): sanitize dynamic href and xlink:href bindings on SVG a elements
Dynamic bindings to `href` and `xlink:href` attributes on SVG `<a>` elements (`<svg:a>`) were previously unmapped in the DOM security schema. As a result, they bypassed sanitization completely, creating a potential XSS vulnerability if bound to untrusted user inputs (e.g., `javascript:` URLs). This fix mitigates this risk by: 1. Registering `href` and `xlink:href` on `<svg:a>` elements under the `SecurityContext.URL` context in both the compiler and core DOM security schemas. 2. Enabling template compilation to output runtime URL sanitization checks (`ɵɵsanitizeUrl`) on these attributes. 3. Adding regression and verification test cases to ensure dynamic SVG link bindings are safely sanitized at runtime while static values are correctly allowed.
1 parent bdda686 commit 2e3d037

4 files changed

Lines changed: 64 additions & 0 deletions

File tree

‎packages/compiler/src/schema/dom_security_schema.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,8 @@ export function SECURITY_SCHEMA(): {[k: string]: SecurityContext} {
115115
['object', ['codebase', 'data']],
116116
]);
117117

118+
registerContext(SecurityContext.URL, SVG_NAMESPACE, [['a', ['href', 'xlink:href']]]);
119+
118120
// Keep this in sync with SECURITY_SENSITIVE_ELEMENTS in packages/core/src/sanitization/sanitization.ts
119121
// Unknown is the internal tag name for unknown elements example used for host-bindings.
120122
// These are unsafe as `attributeName` can be `href` or `xlink:href`

‎packages/compiler/test/schema/dom_element_schema_registry_spec.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,12 @@ If 'onAnything' is a directive input, make sure the directive is imported by the
167167
expect(registry.securityContext(':svg:set', 'to', false)).toBe(
168168
SecurityContext.ATTRIBUTE_NO_BINDING,
169169
);
170+
171+
// SVG link attributes
172+
expect(registry.securityContext(':svg:a', 'href', false)).toBe(SecurityContext.URL);
173+
expect(registry.securityContext(':svg:a', 'xlink:href', false)).toBe(SecurityContext.URL);
174+
expect(registry.securityContext(':svg:a', 'href', true)).toBe(SecurityContext.URL);
175+
expect(registry.securityContext(':svg:a', 'xlink:href', true)).toBe(SecurityContext.URL);
170176
});
171177

172178
it('should detect properties on namespaced elements', () => {

‎packages/core/src/sanitization/dom_security_schema.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,8 @@ export function SECURITY_SCHEMA(): {[k: string]: SecurityContext} {
121121
['script', ['src', 'href', 'xlink:href']],
122122
]);
123123

124+
registerContext(SecurityContext.URL, SVG_NAMESPACE, [['a', ['href', 'xlink:href']]]);
125+
124126
// Keep this in sync with SECURITY_SENSITIVE_ELEMENTS in packages/core/src/sanitization/sanitization.ts
125127
// The `unknown` elements refer to cases when we need to validate the input/binding in a directive (host bindings)
126128
// and the directive can be applied to multiple different elements (with different tag names). In this case we generate

‎packages/core/test/acceptance/security_spec.ts‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -902,3 +902,57 @@ describe('SVG <script> bindings', () => {
902902
expect(fixture.nativeElement.querySelector('script')).toBeFalsy();
903903
});
904904
});
905+
906+
describe('SVG <a> link sanitization', () => {
907+
it('should sanitize dynamic `href` bindings on <svg:a>', () => {
908+
@Component({
909+
template: '<svg><a [attr.href]="url"></a></svg>',
910+
changeDetection: ChangeDetectionStrategy.Eager,
911+
})
912+
class TestCmp {
913+
url = 'javascript:alert(1)';
914+
}
915+
916+
const fixture = TestBed.createComponent(TestCmp);
917+
fixture.detectChanges();
918+
919+
const link = fixture.nativeElement.querySelector('a');
920+
expect(link.getAttribute('href')).toEqual('unsafe:javascript:alert(1)');
921+
});
922+
923+
it('should sanitize dynamic `xlink:href` bindings on <svg:a>', () => {
924+
@Component({
925+
template: '<svg><a [attr.xlink:href]="url"></a></svg>',
926+
changeDetection: ChangeDetectionStrategy.Eager,
927+
})
928+
class TestCmp {
929+
url = 'javascript:alert(1)';
930+
}
931+
932+
const fixture = TestBed.createComponent(TestCmp);
933+
fixture.detectChanges();
934+
935+
const link = fixture.nativeElement.querySelector('a');
936+
expect(link.getAttribute('xlink:href')).toEqual('unsafe:javascript:alert(1)');
937+
});
938+
939+
it('should allow static unsafe `href` and `xlink:href` on <svg:a>', () => {
940+
@Component({
941+
template: `
942+
<svg>
943+
<a href="javascript:alert(1)"></a>
944+
<a xlink:href="javascript:alert(2)"></a>
945+
</svg>
946+
`,
947+
changeDetection: ChangeDetectionStrategy.Eager,
948+
})
949+
class TestCmp {}
950+
951+
const fixture = TestBed.createComponent(TestCmp);
952+
fixture.detectChanges();
953+
954+
const links = fixture.nativeElement.querySelectorAll('a');
955+
expect(links[0].getAttribute('href')).toEqual('javascript:alert(1)');
956+
expect(links[1].getAttribute('xlink:href')).toEqual('javascript:alert(2)');
957+
});
958+
});

0 commit comments

Comments
 (0)