Skip to content

Commit b89b0a8

Browse files
crisbetomattrbeck
authored andcommitted
fix(core): sanitize translated attribute bindings with interpolations
Fixes that we weren't sanitizing attribute bindings with interpolations if they're marked for translation, for example: `<a href="{{evilLink}}" i18n-href></a>`. Also adds a bit more test coverage for our sanitization.
1 parent a554759 commit b89b0a8

File tree

2 files changed

+74
-13
lines changed

2 files changed

+74
-13
lines changed

packages/core/src/render3/i18n/i18n_parse.ts

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@ export function i18nAttributesFirstPass(tView: TView, index: number, values: str
388388
previousElementIndex,
389389
attrName,
390390
countBindings(updateOpCodes),
391-
null,
391+
URI_ATTRS[attrName.toLowerCase()] ? _sanitizeUrl : null,
392392
);
393393
}
394394
}
@@ -810,18 +810,14 @@ function walkIcuTree(
810810
const hasBinding = !!attr.value.match(BINDING_REGEXP);
811811
if (hasBinding) {
812812
if (VALID_ATTRS.hasOwnProperty(lowerAttrName)) {
813-
if (URI_ATTRS[lowerAttrName]) {
814-
generateBindingUpdateOpCodes(
815-
update,
816-
attr.value,
817-
newIndex,
818-
attr.name,
819-
0,
820-
_sanitizeUrl,
821-
);
822-
} else {
823-
generateBindingUpdateOpCodes(update, attr.value, newIndex, attr.name, 0, null);
824-
}
813+
generateBindingUpdateOpCodes(
814+
update,
815+
attr.value,
816+
newIndex,
817+
attr.name,
818+
0,
819+
URI_ATTRS[lowerAttrName] ? _sanitizeUrl : null,
820+
);
825821
} else {
826822
ngDevMode &&
827823
console.warn(

packages/core/test/acceptance/i18n_spec.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3605,6 +3605,71 @@ describe('runtime i18n', () => {
36053605
'translatedText value',
36063606
);
36073607
});
3608+
3609+
describe('attribute sanitization', () => {
3610+
@Component({template: ''})
3611+
class SanitizeAppComp {
3612+
url = 'javascript:alert("oh no")';
3613+
count = 0;
3614+
}
3615+
3616+
it('should sanitize translated attribute binding', () => {
3617+
const fixture = initWithTemplate(SanitizeAppComp, '<a [attr.href]="url" i18n-href></a>');
3618+
const link: HTMLAnchorElement = fixture.nativeElement.querySelector('a');
3619+
expect(link.getAttribute('href')).toMatch(/^unsafe:/);
3620+
});
3621+
3622+
it('should sanitize translated property binding', () => {
3623+
const fixture = initWithTemplate(SanitizeAppComp, '<a [href]="url" i18n-href></a>');
3624+
const link: HTMLAnchorElement = fixture.nativeElement.querySelector('a');
3625+
expect(link.getAttribute('href')).toMatch(/^unsafe:/);
3626+
});
3627+
3628+
it('should sanitize translated interpolation', () => {
3629+
const fixture = initWithTemplate(SanitizeAppComp, '<a href="{{url}}" i18n-href></a>');
3630+
const link: HTMLAnchorElement = fixture.nativeElement.querySelector('a');
3631+
expect(link.getAttribute('href')).toMatch(/^unsafe:/);
3632+
});
3633+
3634+
it('should sanitize interpolation inside translated element', () => {
3635+
const fixture = initWithTemplate(SanitizeAppComp, `<div i18n><a href="{{url}}"></a></div>`);
3636+
const link: HTMLAnchorElement = fixture.nativeElement.querySelector('a');
3637+
expect(link.getAttribute('href')).toMatch(/^unsafe:/);
3638+
});
3639+
3640+
it('should sanitize attribute binding inside translated element', () => {
3641+
const fixture = initWithTemplate(
3642+
SanitizeAppComp,
3643+
`<div i18n><a [attr.href]="url"></a></div>`,
3644+
);
3645+
const link: HTMLAnchorElement = fixture.nativeElement.querySelector('a');
3646+
expect(link.getAttribute('href')).toMatch(/^unsafe:/);
3647+
});
3648+
3649+
it('should sanitize property binding inside translated element', () => {
3650+
const fixture = initWithTemplate(SanitizeAppComp, `<div i18n><a [href]="url"></a></div>`);
3651+
const link: HTMLAnchorElement = fixture.nativeElement.querySelector('a');
3652+
expect(link.getAttribute('href')).toMatch(/^unsafe:/);
3653+
});
3654+
3655+
it('should sanitize property binding inside an ICU', () => {
3656+
const fixture = initWithTemplate(
3657+
SanitizeAppComp,
3658+
`<div i18n>{count, plural,
3659+
=0 {no <strong>link</strong> yet}
3660+
other {{{count}} Here is the <a href="{{url}}">link</a>!}
3661+
}</div>`,
3662+
);
3663+
3664+
expect(fixture.nativeElement.querySelector('a')).toBeFalsy();
3665+
3666+
fixture.componentInstance.count = 1;
3667+
fixture.detectChanges();
3668+
const link: HTMLAnchorElement = fixture.nativeElement.querySelector('a');
3669+
expect(link).toBeTruthy();
3670+
expect(link.getAttribute('href')).toMatch(/^unsafe:/);
3671+
});
3672+
});
36083673
});
36093674

36103675
function initWithTemplate(compType: Type<any>, template: string) {

0 commit comments

Comments
 (0)