Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sanitize dynamic tags #5615

Merged
merged 3 commits into from
Dec 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fresh-bats-prove.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Sanitize dynamically rendered tags to strip out any attributes
14 changes: 11 additions & 3 deletions packages/astro/src/runtime/server/render/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '';
Expand Down Expand Up @@ -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) {
Expand Down
65 changes: 65 additions & 0 deletions packages/astro/test/units/render/components.test.js
Original file line number Diff line number Diff line change
@@ -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);
}
);
});
});