Sanitize dynamic tags (#5615)

natemoo-re · web-flow · commit d85ec7484ce1 · 2022-12-16T13:14:42.000-06:00
* fix: sanitize tags

* fix: better element sanitization

* chore: remove unused import

Co-authored-by: Nate Moore &lt;nate@astro.build&gt;
diff --git a/.changeset/fresh-bats-prove.md b/.changeset/fresh-bats-prove.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Sanitize dynamically rendered tags to strip out any attributes
diff --git a/packages/astro/src/runtime/server/render/component.ts b/packages/astro/src/runtime/server/render/component.ts
@@ -239,12 +239,14 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr
 	// This is a custom element without a renderer. Because of that, render it
 	// as a string and the user is responsible for adding a script tag for the component definition.
 	if (!html && typeof Component === 'string') {
+		// Sanitize tag name because some people might try to inject attributes 🙄
+		const Tag = sanitizeElementName(Component);
 		const childSlots = Object.values(children).join('');
 		const iterable = renderAstroTemplateResult(
-			await renderTemplate`<${Component}${internalSpreadAttributes(props)}${markHTMLString(
-				childSlots === '' && voidElementNames.test(Component)
+			await renderTemplate`<${Tag}${internalSpreadAttributes(props)}${markHTMLString(
+				childSlots === '' && voidElementNames.test(Tag)
 					? `/>`
-					: `>${childSlots}</${Component}>`
+					: `>${childSlots}</${Tag}>`
 			)}`
 		);
 		html = '';
@@ -322,6 +324,12 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr
 	return renderAll();
 }
 
+function sanitizeElementName(tag: string) {
+	const unsafe = /[&<>'"\s]+/g;
+	if (!unsafe.test(tag)) return tag;
+	return tag.trim().split(unsafe)[0].trim();
+}
+
 async function renderFragmentComponent(result: SSRResult, slots: any = {}) {
 	const children = await renderSlot(result, slots?.default);
 	if (children == null) {
diff --git a/packages/astro/test/units/render/components.test.js b/packages/astro/test/units/render/components.test.js
@@ -0,0 +1,65 @@
+import { expect } from 'chai';
+import * as cheerio from 'cheerio';
+
+import { runInContainer } from '../../../dist/core/dev/index.js';
+import { createFs, createRequestAndResponse } from '../test-utils.js';
+import svelte from '../../../../integrations/svelte/dist/index.js';
+import { defaultLogging } from '../../test-utils.js';
+
+const root = new URL('../../fixtures/alias/', import.meta.url);
+
+describe('core/render components', () => {
+	it('should sanitize dynamic tags', async () => {
+		const fs = createFs(
+			{
+				'/src/pages/index.astro': `
+				---
+				const TagA = 'p style=color:red;'
+				const TagB = 'p><script id="pwnd">console.log("pwnd")</script>'
+				---
+				<html>
+					<head><title>testing</title></head>
+					<body>
+						<TagA id="target" />
+						<TagB />
+					</body>
+				</html>
+			`,
+			},
+			root
+		);
+
+		await runInContainer(
+			{
+				fs,
+				root,
+				logging: {
+					...defaultLogging,
+					// Error is expected in this test
+					level: 'silent',
+				},
+				userConfig: {
+					integrations: [svelte()],
+				},
+			},
+			async (container) => {
+				const { req, res, done, text } = createRequestAndResponse({
+					method: 'GET',
+					url: '/',
+				});
+				container.handle(req, res);
+
+				await done;
+				const html = await text();
+				const $ = cheerio.load(html);
+				const target = $('#target');
+
+				expect(target).not.to.be.undefined;
+				expect(target.attr('id')).to.equal('target');
+				expect(target.attr('style')).to.be.undefined;
+
+				expect($('#pwnd').length).to.equal(0);
+			}
+		);
+	});
+});

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +'astro': patch
 +---
++
 +Sanitize dynamically rendered tags to strip out any attributes