Skip to content

Commit 5240e26

Browse files
authored
Validate attribute names on custom HTML elements during SSR (#17251)
* Harden renderHTMLElement to drop invalid attribute names * update changeset * Apply suggestion from @matthewp
1 parent fee9069 commit 5240e26

4 files changed

Lines changed: 63 additions & 3 deletions

File tree

.changeset/quick-otters-attend.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'astro': patch
3+
---
4+
5+
Hardens the handling of attribute rendering when using with custom elements.

packages/astro/src/runtime/server/render/dom.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { SSRResult } from '../../../types/public/internal.js';
22
import { markHTMLString } from '../escape.js';
33
import { renderSlotToString } from './slot.js';
4-
import { toAttributeString } from './util.js';
4+
import { INVALID_ATTR_NAME_CHAR, toAttributeString } from './util.js';
55

66
export function componentIsHTMLElement(Component: unknown) {
77
return typeof HTMLElement !== 'undefined' && HTMLElement.isPrototypeOf(Component as object);
@@ -18,6 +18,12 @@ export async function renderHTMLElement(
1818
let attrHTML = '';
1919

2020
for (const attr in props) {
21+
// Reject attribute names with characters that could break out of the attribute context.
22+
// Without this guard, untrusted prop keys spread onto the element could inject arbitrary
23+
// markup or event-handler attributes (XSS). Mirrors the guard in `addAttribute`.
24+
if (INVALID_ATTR_NAME_CHAR.test(attr)) {
25+
continue;
26+
}
2127
attrHTML += ` ${attr}="${toAttributeString(await props[attr])}"`;
2228
}
2329

packages/astro/src/runtime/server/render/util.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ const DOUBLE_QUOTE_REGEX = /"/g;
1515
const STATIC_DIRECTIVES = new Set(['set:html', 'set:text']);
1616

1717
// Per the HTML spec, attribute names must not contain ASCII whitespace, ", ', >, /, or =.
18-
const INVALID_ATTR_NAME_CHAR = /[\s"'>/=]/;
18+
export const INVALID_ATTR_NAME_CHAR = /[\s"'>/=]/;
1919

2020
// converts (most) arbitrary strings to valid JS identifiers
2121
const toIdent = (k: string) =>

packages/astro/test/units/render/html-primitives.test.ts

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import assert from 'node:assert/strict';
2-
import { describe, it } from 'node:test';
2+
import { after, before, describe, it } from 'node:test';
33
import * as cheerio from 'cheerio';
44
import {
55
addAttribute,
@@ -15,6 +15,7 @@ import {
1515
Fragment,
1616
render as renderTemplate,
1717
renderComponent,
18+
renderHTMLElement,
1819
renderSlot,
1920
unescapeHTML,
2021
} from '../../../dist/runtime/server/index.js';
@@ -263,6 +264,54 @@ describe('spreadAttributes rejects invalid attribute keys', () => {
263264
});
264265
});
265266

267+
describe('renderHTMLElement rejects invalid attribute keys', () => {
268+
// renderHTMLElement resolves the tag name through customElements.getName().
269+
// In a Node test environment this global doesn't exist, so we stub it.
270+
const originalCustomElements = globalThis.customElements;
271+
const result = {} as any;
272+
273+
before(() => {
274+
globalThis.customElements = {
275+
getName: () => 'my-el',
276+
} as any;
277+
});
278+
279+
after(() => {
280+
globalThis.customElements = originalCustomElements;
281+
});
282+
283+
it('drops malicious keys while keeping valid ones', async () => {
284+
const html = await renderHTMLElement(
285+
result,
286+
class {} as any,
287+
{
288+
'onmouseover=alert(document.domain) x': 'y',
289+
'x><script>alert(1)</script>': 'z',
290+
'data-safe': 'ok',
291+
},
292+
{},
293+
);
294+
const output = String(html);
295+
assert.ok(output.includes('data-safe="ok"'));
296+
assert.ok(!output.includes('onmouseover'));
297+
assert.ok(!output.includes('<script>'));
298+
assert.ok(!output.includes('alert'));
299+
});
300+
301+
it('preserves namespaced and normal attribute names', async () => {
302+
const html = await renderHTMLElement(
303+
result,
304+
class {} as any,
305+
{ 'id': 'a', 'data-foo': 'b', 'on:click': 'c' },
306+
{},
307+
);
308+
const output = String(html);
309+
assert.ok(output.includes('id="a"'));
310+
assert.ok(output.includes('data-foo="b"'));
311+
assert.ok(output.includes('on:click="c"'));
312+
});
313+
});
314+
266315
describe('Correctly serializes boolean attributes (#astro-basic)', async () => {
267316
// h1 data-something and h2 not-data-ok are both empty-string boolean-ish attrs
268317
it('renders empty-value attribute for data-* attr with no value', () => {

0 commit comments

Comments
 (0)