feat(core): support prefix-insensitive DOM schema lookups and compile-time i18n attribute validation#68591
Conversation
c004f1b to
cee01f9
Compare
| // value of that attribute is updated to read the extracted i18n variable. | ||
| const attributesForMessage = extractedAttributesByI18nContext.get(op.i18nContext); | ||
| if (attributesForMessage !== undefined) { | ||
| for (const attr of attributesForMessage) { |
There was a problem hiding this comment.
Should we run the santizer here or outright reject this in translations for such attributes?
There was a problem hiding this comment.
Thanks Alan. To keep this aligned with the discussion from #68559: I agree the right fix should be centralized and schema-driven, not split into per-attribute patches.
My main concern is preserving the security invariant consistently:
translated static i18n attributes should not bypass the sanitizer/validator path Angular applies to the same security-sensitive tag/attribute pair elsewhere.
So I think the expected behavior should be:
- non-security-sensitive translated attributes remain allowed;
URL/HTML/STYLEcontexts are sanitized;RESOURCE_URLfails closed, because translated strings should not become trusted executable resource URLs;ATTRIBUTE_NO_BINDINGgoes throughvalidateAttribute/ rejection, because those attributes are not safely sanitizable.
The current #68591 direction appears to cover the concrete cases I reported across #68557, #68559, #68560, and #68580. My priority is to help make the broader hardening complete and robust rather than keep fragmented fixes open.
The only extra hardening point I would keep an eye on is namespace normalization anywhere SECURITY_SCHEMA is queried, especially SVG/MathML paths such as :svg:script, :svg:set, and MathML href / xlink:href, plus avoiding drift between the compiler-side and runtime/core schema
copies.
Hope this helps you and the team. Happy to test any extra cases if useful.
| if (!ir.isStringLiteral(value)) { | ||
| throw Error('AssertionError: extracted attribute value should be string literal'); | ||
| } | ||
| if (trustedValueFn !== null && ir.isStringLiteral(value)) { |
There was a problem hiding this comment.
Is this change relevant/required?
b6efa18 to
f54d970
Compare
| } | ||
|
|
||
| if (typeof validationConfig !== 'boolean') { | ||
| if (!tNode) { |
There was a problem hiding this comment.
I'm curious about the !tNode condition- why does lack of aTNode` mean something from a security standpoint?
There was a problem hiding this comment.
From a security standpoint, a TNode represents a safely tracked, statically verified element.
If attribute validation is triggered dynamically via a runtime binding instruction but the TNode is missing, it implies an untracked assignment such as arbitrary renderer calls or unverified directive host attribute bindings. Because we cannot inspect the physical template element in these contexts to confirm the target DOM state is safe (e.g., verifying what property an tag is targeting), it treats the dynamic assignment as a potential injection risk. This is because static i18n attributes are evaluated early inside the consts array definition before any elements are created.
| cmd = "cp $< $@", | ||
| ) | ||
|
|
||
| write_source_file( |
There was a problem hiding this comment.
@alxhub, I went with the bazel approach. This under the hood has also a diff test.
|
Caretaker note: this requires a G3 patch https://critique.corp.google.com/cl/917087656 |
alxhub
left a comment
There was a problem hiding this comment.
Reviewed-for: fw-general, fw-compiler, fw-security
…time i18n attribute validation Updates `DomElementSchemaRegistry` to strip `:svg:` and `:math:` namespace prefixes from tag names before querying `SECURITY_SCHEMA` at compile-time. This allows SVG and MathML attributes to correctly match their security contexts during compilation. Also implements dynamic schema-based sanitization and validation for static i18n attributes by wrapping `` expressions directly in their core sanitizers and validators at compile-time via an optimized `switch` statement. Additionally, updates the runtime i18n parser to dynamically resolve sanitizers based on the `SECURITY_SCHEMA` registry and cleans up type assertions inside `ɵɵvalidateAttribute` to safely support evaluation when no `TNode` is selected.
…ompile-time i18n attribute validation
…ompile-time i18n attribute validation
…ompile-time i18n attribute validation
…ompile-time i18n attribute validation
…ompile-time i18n attribute validation
josephperrott
left a comment
There was a problem hiding this comment.
LGTM
Reviewed-for: fw-security
AndrewKushnir
left a comment
There was a problem hiding this comment.
Reviewed-for: size-tracking
Updates
DomElementSchemaRegistryto strip:svg:and:math:namespace prefixes from tag names before queryingSECURITY_SCHEMAat compile-time. This allows SVG and MathML attributes to correctly match their security contexts during compilation.Also implements dynamic schema-based sanitization and validation for static i18n attributes by wrapping `` expressions directly in their core sanitizers and validators at compile-time via an optimized
switchstatement.Additionally, updates the runtime i18n parser to dynamically resolve sanitizers based on the
SECURITY_SCHEMAregistry and cleans up type assertions insideɵɵvalidateAttributeto safely support evaluation when noTNodeis selected.