Skip to content

Commit 7a05a9a

Browse files
alan-agius4alxhub
authored andcommitted
fix(core): validate security-sensitive attributes in i18n bindings
Ensures that security-sensitive attributes (e.g., sandbox, allow) are correctly validated when applied through i18n-* dynamic attribute bindings, preventing potential policy bypasses. Closes #68418 (cherry picked from commit 9d7a609)
1 parent dc9b40e commit 7a05a9a

3 files changed

Lines changed: 58 additions & 3 deletions

File tree

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

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ import {
1717
} from '../../sanitization/html_sanitizer';
1818
import {getInertBodyHelper} from '../../sanitization/inert_body';
1919
import {_sanitizeUrl} from '../../sanitization/url_sanitizer';
20+
import {
21+
ɵɵvalidateAttribute as _validateAttribute,
22+
SECURITY_SENSITIVE_ELEMENTS,
23+
} from '../../sanitization/sanitization';
2024
import {
2125
assertDefined,
2226
assertEqual,
@@ -388,7 +392,7 @@ export function i18nAttributesFirstPass(tView: TView, index: number, values: str
388392
previousElementIndex,
389393
attrName,
390394
countBindings(updateOpCodes),
391-
SENSITIVE_ATTRS[attrName.toLowerCase()] ? _sanitizeUrl : null,
395+
i18nSanitizeAttribute(attrName),
392396
);
393397
}
394398
}
@@ -816,7 +820,7 @@ function walkIcuTree(
816820
newIndex,
817821
attr.name,
818822
0,
819-
SENSITIVE_ATTRS[lowerAttrName] ? _sanitizeUrl : null,
823+
i18nSanitizeAttribute(lowerAttrName),
820824
);
821825
} else {
822826
ngDevMode &&
@@ -969,3 +973,33 @@ function addCreateAttribute(
969973
) {
970974
create.push((newIndex << IcuCreateOpCode.SHIFT_REF) | IcuCreateOpCode.Attr, attrName, attrValue);
971975
}
976+
977+
/**
978+
* Caches all keys of `SECURITY_SENSITIVE_ELEMENTS` in a Set to avoid recomputing
979+
* or scanning them on every invocation.
980+
*/
981+
const SECURITY_SENSITIVE_ATTRS: ReadonlySet<string> = /* @__PURE__ */ (() =>
982+
new Set(
983+
Object.values(SECURITY_SENSITIVE_ELEMENTS).flatMap((attrs) =>
984+
attrs ? Object.keys(attrs) : [],
985+
),
986+
))();
987+
988+
/**
989+
* Returns a sanitizer for the given attribute name or null if the attribute is not security sensitive.
990+
*
991+
* @param attrName The name of the attribute to sanitize.
992+
* @returns The sanitizer for the given attribute name.
993+
*/
994+
function i18nSanitizeAttribute(attrName: string): SanitizerFn | null {
995+
const lowerAttrName = attrName.toLowerCase();
996+
if (SENSITIVE_ATTRS[lowerAttrName]) {
997+
return _sanitizeUrl;
998+
}
999+
1000+
if (SECURITY_SENSITIVE_ATTRS.has(lowerAttrName)) {
1001+
return _validateAttribute;
1002+
}
1003+
1004+
return null;
1005+
}

packages/core/src/sanitization/sanitization.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ const SECURITY_SENSITIVE_ATTRIBUTE_NAMES: ReadonlySet<string> = new Set(['href',
291291
* @remarks Keep this in sync with DOM Security Schema.
292292
* @see [SECURITY_SCHEMA](../../../compiler/src/schema/dom_security_schema.ts)
293293
*/
294-
const SECURITY_SENSITIVE_ELEMENTS: Record<
294+
export const SECURITY_SENSITIVE_ELEMENTS: Record<
295295
string,
296296
Record<string, true | undefined | ReadonlySet<string>> | undefined
297297
> = {

packages/core/test/acceptance/security_spec.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,27 @@ describe('iframe processing', () => {
295295
});
296296
});
297297

298+
it('should error when a translated security-sensitive attribute contains bindings', () => {
299+
@Component({
300+
selector: 'my-comp',
301+
template: `
302+
<iframe
303+
src="${TEST_IFRAME_URL}"
304+
i18n-sandbox
305+
sandbox="allow-forms {{ extraPrivileges }}"
306+
>
307+
</iframe>
308+
`,
309+
310+
changeDetection: ChangeDetectionStrategy.Eager,
311+
})
312+
class IframeComp {
313+
extraPrivileges = 'allow-scripts allow-same-origin';
314+
}
315+
316+
expectIframeCreationToFail(IframeComp);
317+
});
318+
298319
it('should work when a directive sets a security-sensitive attribute as a static attribute', () => {
299320
@Directive({
300321
selector: '[dir]',

0 commit comments

Comments
 (0)