Visitar URL original
fix(platform-browser): disallow event handler attributes in Meta · angular/angular@6f9a6be · GitHub
Skip to content

Commit 6f9a6be

Browse files
SkyZeroZxSkyZeroZx
authored
fix(platform-browser): disallow event handler attributes in Meta
Prevent arbitrary MetaDefinition properties from writing on* handlers directly to meta elements. Browser events can execute these handlers, including on meta elements rendered in the document body.
1 parent 3f8d9d6 commit 6f9a6be

4 files changed

Lines changed: 96 additions & 3 deletions

File tree

‎goldens/public-api/platform-browser/errors.api.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ export const enum RuntimeErrorCode {
1515
// (undocumented)
1616
INSERT_BEFORE_NODE_NOT_FOUND = -5106,
1717
// (undocumented)
18+
INVALID_EVENT_ATTRIBUTE = 5203,
19+
// (undocumented)
1820
NO_PLUGIN_FOR_EVENT = -5101,
1921
// (undocumented)
2022
ROOT_NODE_NOT_FOUND = -5104,

‎packages/platform-browser/src/browser/meta.ts‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,14 @@
77
*/
88

99
import {DOCUMENT, ɵgetDOM as getDOM} from '@angular/common';
10-
import {inject, Service} from '@angular/core';
10+
import {inject, ɵRuntimeError as RuntimeError, Service} from '@angular/core';
11+
12+
import {RuntimeErrorCode} from '../errors';
1113

1214
/**
1315
* Represents the attributes of an HTML `<meta>` element. The element itself is
14-
* represented by the internal `HTMLMetaElement`.
16+
* represented by the internal `HTMLMetaElement`. Event handler attributes, which start with
17+
* `on`, are not allowed.
1518
*
1619
* @see [HTML meta tag](https://developer.mozilla.org/docs/Web/HTML/Element/meta)
1720
* @see {@link Meta}
@@ -124,6 +127,7 @@ export class Meta {
124127
* @return The modified element.
125128
*/
126129
updateTag(tag: MetaDefinition, selector?: string): HTMLMetaElement | null {
130+
validateMetaDefinition(tag);
127131
selector ??= parseSelector(tag);
128132
const meta = this.getTag(selector);
129133
if (meta) {
@@ -156,6 +160,7 @@ export class Meta {
156160
meta: MetaDefinition,
157161
forceCreation: boolean = false,
158162
): HTMLMetaElement {
163+
validateMetaDefinition(meta);
159164
if (!forceCreation) {
160165
const selector: string = parseSelector(meta);
161166
// It's allowed to have multiple elements with the same name so it's not enough to
@@ -180,6 +185,19 @@ function setMetaElementAttributes(tag: MetaDefinition, el: HTMLMetaElement) {
180185
Object.keys(tag).forEach((prop: string) => el.setAttribute(getMetaKeyMap(prop), tag[prop]));
181186
}
182187

188+
function validateMetaDefinition(tag: MetaDefinition): void {
189+
for (const prop of Object.keys(tag)) {
190+
const attributeName = getMetaKeyMap(prop);
191+
if (attributeName.toLowerCase().startsWith('on')) {
192+
throw new RuntimeError(
193+
RuntimeErrorCode.INVALID_EVENT_ATTRIBUTE,
194+
(typeof ngDevMode === 'undefined' || ngDevMode) &&
195+
`The Meta service does not allow setting event handler attribute '${attributeName}' for security reasons.`,
196+
);
197+
}
198+
}
199+
}
200+
183201
function parseSelector(tag: MetaDefinition): string {
184202
const attr: string = tag.name ? 'name' : 'property';
185203
return `${attr}=${escapeSelectorValue(String(tag[attr]))}`;
@@ -196,7 +214,7 @@ function containsAttributes(tag: MetaDefinition, elem: HTMLMetaElement): boolean
196214
}
197215

198216
function getMetaKeyMap(prop: string): string {
199-
return META_KEYS_MAP[prop] || prop;
217+
return Object.hasOwn(META_KEYS_MAP, prop) ? META_KEYS_MAP[prop] : prop;
200218
}
201219

202220
function isMetaTag(tag: HTMLElement | null): tag is HTMLMetaElement {

‎packages/platform-browser/src/errors.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ export const enum RuntimeErrorCode {
2727
SANITIZATION_UNSAFE_SCRIPT = 5200,
2828
SANITIZATION_UNSAFE_RESOURCE_URL = -5201,
2929
SANITIZATION_UNEXPECTED_CTX = 5202,
30+
INVALID_EVENT_ATTRIBUTE = 5203,
3031

3132
// Animations related errors (5300-5400 range)
3233
ANIMATION_RENDERER_ASYNC_LOADING_FAILURE = 5300,

‎packages/platform-browser/test/browser/meta_spec.ts‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,26 @@ describe('Meta service', () => {
9090
expect(actual!.getAttribute('content')).toEqual('4321');
9191
});
9292

93+
it('should reject event handler attributes targeting a body meta tag', () => {
94+
doc.body.appendChild(defaultMeta);
95+
96+
const evil = 'alert(1)';
97+
98+
expect(metaService.getTag('property="fb:app_id"')).toBe(defaultMeta);
99+
expect(() =>
100+
metaService.updateTag({
101+
property: 'fb:app_id',
102+
style: 'content-visibility:auto',
103+
oncontentvisibilityautostatechange: evil,
104+
}),
105+
).toThrowError(
106+
/NG05203: The Meta service does not allow setting event handler attribute 'oncontentvisibilityautostatechange'/,
107+
);
108+
109+
expect(defaultMeta.getAttribute('style')).toBeNull();
110+
expect(defaultMeta.getAttribute('oncontentvisibilityautostatechange')).toBeNull();
111+
});
112+
93113
it('should not allow a custom selector to match off target elements like the body tag', () => {
94114
// This payload attempts to break out of the `meta[name="..."]` constraint entirely
95115
// and inject a comma to target arbitrary DOM elements like the `body` tag via the
@@ -144,6 +164,42 @@ describe('Meta service', () => {
144164
metaService.removeTagElement(actual);
145165
});
146166

167+
it('should reject event handler attributes without adding a tag', () => {
168+
const selector = 'name="og:title"';
169+
const evil = 'alert(1)';
170+
171+
expect(() =>
172+
metaService.addTag({
173+
name: 'og:title',
174+
style: 'content-visibility:auto',
175+
oncontentvisibilityautostatechange: evil,
176+
}),
177+
).toThrowError(
178+
/NG05203: The Meta service does not allow setting event handler attribute 'oncontentvisibilityautostatechange'/,
179+
);
180+
181+
expect(metaService.getTag(selector)).toBeNull();
182+
});
183+
184+
it('should reject event handler attributes in addTags without adding tags', () => {
185+
const selector = 'name="og:title"';
186+
const evil = 'alert(1)';
187+
188+
expect(() =>
189+
metaService.addTags([
190+
{
191+
name: 'og:title',
192+
style: 'content-visibility:auto',
193+
oncontentvisibilityautostatechange: evil,
194+
},
195+
]),
196+
).toThrowError(
197+
/NG05203: The Meta service does not allow setting event handler attribute 'oncontentvisibilityautostatechange'/,
198+
);
199+
200+
expect(metaService.getTag(selector)).toBeNull();
201+
});
202+
147203
it('should add httpEquiv meta tag as http-equiv', () => {
148204
metaService.addTag({httpEquiv: 'refresh', content: '3;url=http://test'});
149205

@@ -156,6 +212,22 @@ describe('Meta service', () => {
156212
metaService.removeTagElement(actual);
157213
});
158214

215+
it('should add attributes whose names match Object prototype keys', () => {
216+
const meta = metaService.addTag({
217+
name: 'prototype-keys',
218+
constructor: 'constructor',
219+
toString: 'toString',
220+
['__proto__']: '__proto__',
221+
})!;
222+
223+
expect(meta.getAttribute('constructor')).toEqual('constructor');
224+
expect(meta.getAttribute('toString')).toEqual('toString');
225+
expect(meta.getAttribute('__proto__')).toEqual('__proto__');
226+
227+
// clean up
228+
metaService.removeTagElement(meta);
229+
});
230+
159231
it('should escape selector values when deriving the match selector', () => {
160232
// This payload attempts to prematurely close the attribute selector
161233
// and match another attribute.

0 commit comments

Comments
 (0)