Improve JSON payload handling

.changeset/fuzzy-donkeys-warn.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+Fix XSS vulnerability on SSR pages with fetched data on `load()`

packages/kit/src/runtime/client/renderer.js

@@ -95,12 +95,12 @@ function create_updated_store() {
  * @param {RequestInit} [opts]
  */
 function initial_fetch(resource, opts) {
-	const url = typeof resource === 'string' ? resource : resource.url;
+	const url = JSON.stringify(typeof resource === 'string' ? resource : resource.url);
 
-	let selector = `script[data-type="svelte-data"][data-url=${JSON.stringify(url)}]`;
+	let selector = `script[sveltekit\\:data-type="data"][sveltekit\\:data-url=${url}]`;
 
 	if (opts && typeof opts.body === 'string') {
-		selector += `[data-body="${hash(opts.body)}"]`;
+		selector += `[sveltekit\\:data-body="${hash(opts.body)}"]`;
 	}
 
 	const script = document.querySelector(selector);
@@ -219,7 +219,7 @@ export class Renderer {
 				let props;
 
 				if (is_leaf) {
-					const serialized = document.querySelector('[data-type="svelte-props"]');
+					const serialized = document.querySelector('script[sveltekit\\:data-type="props"]');
 					if (serialized) {
 						props = JSON.parse(/** @type {string} */ (serialized.textContent));
 					}

packages/kit/src/runtime/server/page/load_node.js

@@ -1,7 +1,5 @@
 import { normalize } from '../../load.js';
 import { respond } from '../index.js';
-import { s } from '../../../utils/misc.js';
-import { escape_json_in_html } from '../../../utils/escape.js';
 import { is_root_relative, resolve } from '../../../utils/url.js';
 import { create_prerendering_url_proxy } from './utils.js';
 import { is_pojo, lowercase_keys, normalize_request_method } from '../utils.js';
@@ -44,13 +42,7 @@ export async function load_node({
 
 	let uses_credentials = false;
 
-	/**
-	 * @type {Array<{
-	 *   url: string;
-	 *   body: string;
-	 *   json: string;
-	 * }>}
-	 */
+	/** @type {Array<import('./types').Fetched>} */
 	const fetched = [];
 
 	/**
@@ -250,8 +242,6 @@ export async function load_node({
 							}
 
 							if (!opts.body || typeof opts.body === 'string') {
-								// the json constructed below is later added to the dom in a script tag
-								// make sure the used values are safe
 								const status_number = Number(response.status);
 								if (isNaN(status_number)) {
 									throw new Error(
@@ -260,11 +250,16 @@ export async function load_node({
 										}" type: ${typeof response.status}`
 									);
 								}
-								// prettier-ignore
+
 								fetched.push({
 									url: requested,
-									body: /** @type {string} */ (opts.body),
-									json: `{"status":${status_number},"statusText":${s(response.statusText)},"headers":${s(headers)},"body":${escape_json_in_html(body)}}`
+									body: opts.body,
+									response: {
+										status: status_number,
+										statusText: response.statusText,
+										headers,
+										body
+									}
 								});
 							}
 

packages/kit/src/runtime/server/page/render.js

@@ -2,7 +2,7 @@ import devalue from 'devalue';
 import { readable, writable } from 'svelte/store';
 import { coalesce_to_error } from '../../../utils/error.js';
 import { hash } from '../../hash.js';
-import { escape_html_attr, escape_json_in_html } from '../../../utils/escape.js';
+import { render_json_payload_script } from '../../../utils/escape.js';
 import { s } from '../../../utils/misc.js';
 import { create_prerendering_url_proxy } from './utils.js';
 import { Csp, csp_ready } from './csp.js';
@@ -57,7 +57,7 @@ export async function render_response({
 	/** @type {Map<string, string>} */
 	const styles = new Map();
 
-	/** @type {Array<{ url: string, body: string, json: string }>} */
+	/** @type {Array<import('./types').Fetched>} */
 	const serialized_data = [];
 
 	let shadow_props;
@@ -253,19 +253,17 @@ export async function render_response({
 
 			body += `\n\t\t<script ${attributes.join(' ')}>${init_app}</script>`;
 
-			// prettier-ignore
 			body += serialized_data
-				.map(({ url, body, json }) => {
-					let attributes = `type="application/json" data-type="svelte-data" data-url=${escape_html_attr(url)}`;
-					if (body) attributes += ` data-body="${hash(body)}"`;
-
-					return `<script ${attributes}>${json}</script>`;
-				})
+				.map(({ url, body, response }) =>
+					render_json_payload_script(
+						{ type: 'data', url, body: typeof body === 'string' ? hash(body) : undefined },
+						response
+					)
+				)
 				.join('\n\t');
 
 			if (shadow_props) {
-				// prettier-ignore
-				body += `<script type="application/json" data-type="svelte-props">${escape_json_in_html(shadow_props)}</script>`;
+				body += render_json_payload_script({ type: 'props' }, shadow_props);
 			}
 		}
 

packages/kit/src/runtime/server/page/types.d.ts

@@ -1,11 +1,22 @@
-import { JSONValue, NormalizedLoadOutput, SSRNode } from 'types';
+import { JSONValue, NormalizedLoadOutput, ResponseHeaders, SSRNode } from 'types';
+
+export type Fetched = {
+	url: string;
+	body?: string | null;
+	response: {
+		status: number;
+		statusText: string;
+		headers: ResponseHeaders;
+		body: string;
+	};
+};
 
 export type Loaded = {
 	node: SSRNode;
 	props: JSONValue | undefined;
 	loaded: NormalizedLoadOutput;
 	stuff: Record<string, any>;
-	fetched: Array<{ url: string; body: string; json: string }>;
+	fetched: Fetched[];
 	set_cookie_headers: string[];
 	uses_credentials: boolean;
 };

packages/kit/src/utils/escape.js

@@ -1,27 +1,59 @@
-// dict from https://github.com/yahoo/serialize-javascript/blob/183c18a776e4635a379fdc620f81771f219832bb/index.js#L25
-/** @type {Record<string, string>} */
-const escape_json_in_html_dict = {
+/**
+ * Inside a script element, only `</script` and `<!--` hold special meaning to the HTML parser.
+ *
+ * The first closes the script element, so everything after is treated as raw HTML.
+ * The second disables further parsing until `-->`, so the script element might be unexpectedly
+ * kept open until until an unrelated HTML comment in the page.
+ *
+ * U+2028 LINE SEPARATOR and U+2029 PARAGRAPH SEPARATOR are escaped for the sake of pre-2018
+ * browsers.
+ *
+ * @see tests for unsafe parsing examples.
+ * @see https://html.spec.whatwg.org/multipage/scripting.html#restrictions-for-contents-of-script-elements
+ * @see https://html.spec.whatwg.org/multipage/syntax.html#cdata-rcdata-restrictions
+ * @see https://html.spec.whatwg.org/multipage/parsing.html#script-data-state
+ * @see https://html.spec.whatwg.org/multipage/parsing.html#script-data-double-escaped-state
+ * @see https://github.com/tc39/proposal-json-superset
+ * @type {Record<string, string>}
+ */
+const render_json_payload_script_dict = {
 	'<': '\\u003C',
-	'>': '\\u003E',
-	'/': '\\u002F',
 	'\u2028': '\\u2028',
 	'\u2029': '\\u2029'
 };
 
-const escape_json_in_html_regex = new RegExp(
-	`[${Object.keys(escape_json_in_html_dict).join('')}]`,
+const render_json_payload_script_regex = new RegExp(
+	`[${Object.keys(render_json_payload_script_dict).join('')}]`,
 	'g'
 );
 
 /**
- * Escape a JSONValue that's going to be embedded in a `<script>` tag
- * @param {import('types').JSONValue} val
+ * Generates a raw HTML string containing a safe script element carrying JSON data and associated attributes.
+ *
+ * It escapes all the special characters needed to guarantee the element is unbroken, but care must
+ * be taken to ensure it is inserted in the document at an acceptable position for a script element,
+ * and that the resulting string isn't further modified.
+ *
+ * Attribute names must be type-checked so we don't need to escape them.
+ *
+ * @param {import('types').PayloadScriptAttributes} attrs A list of attributes to be added to the element.
+ * @param {import('types').JSONValue} payload The data to be carried by the element. Must be serializable to JSON.
+ * @returns {string} The raw HTML of a script element carrying the JSON payload.
+ * @example const html = render_json_payload_script({ type: 'data', url: '/data.json' }, { foo: 'bar' });
  */
-export function escape_json_in_html(val) {
-	return JSON.stringify(val).replace(
-		escape_json_in_html_regex,
-		(match) => escape_json_in_html_dict[match]
+export function render_json_payload_script(attrs, payload) {
+	const safe_payload = JSON.stringify(payload).replace(
+		render_json_payload_script_regex,
+		(match) => render_json_payload_script_dict[match]
 	);
+
+	let safe_attrs = '';
+	for (const [key, value] of Object.entries(attrs)) {
+		if (value === undefined) continue;
+		safe_attrs += ` sveltekit:data-${key}=${escape_html_attr(value)}`;
+	}
+
+	return `<script type="application/json"${safe_attrs}>${safe_payload}</script>`;
 }
 
 /**

packages/kit/src/utils/escape.spec.js

@@ -1,6 +1,35 @@
 import { suite } from 'uvu';
 import * as assert from 'uvu/assert';
-import { escape_html_attr } from './escape.js';
+import { render_json_payload_script, escape_html_attr } from './escape.js';
+
+const json = suite('render_json_payload_script');
+
+json('escapes slashes', () => {
+	assert.equal(
+		render_json_payload_script({ type: 'props' }, { unsafe: '</script><script>alert("xss")' }),
+		'<script type="application/json" sveltekit:data-type="props">' +
+			'{"unsafe":"\\u003C/script>\\u003Cscript>alert(\\"xss\\")"}' +
+			'</script>'
+	);
+});
+
+json('escapes exclamation marks', () => {
+	assert.equal(
+		render_json_payload_script({ type: 'props' }, { '<!--</script>...-->alert("xss")': 'unsafe' }),
+		'<script type="application/json" sveltekit:data-type="props">' +
+			'{"\\u003C!--\\u003C/script>...-->alert(\\"xss\\")":"unsafe"}' +
+			'</script>'
+	);
+});
+
+json('escapes the attribute values', () => {
+	const raw = 'an "attr" & a \ud800';
+	const escaped = 'an &quot;attr&quot; &amp; a &#55296;';
+	assert.equal(
+		render_json_payload_script({ type: 'data', url: raw }, {}),
+		`<script type="application/json" sveltekit:data-type="data" sveltekit:data-url="${escaped}">{}</script>`
+	);
+});
 
 const attr = suite('escape_html_attr');
 
@@ -21,4 +50,5 @@ attr('escapes invalid surrogates', () => {
 	assert.equal(escape_html_attr('\ud800\ud800\udc00\udc00'), '"&#55296;\ud800\udc00&#56320;"');
 });
 
+json.run();
 attr.run();

packages/kit/test/apps/amp/test/test.js

@@ -12,7 +12,7 @@ test('amp is true', async ({ page, baseURL }) => {
 	await expect(page.locator('p')).toHaveText('amp is true');
 
 	// should not include serialized data
-	expect(await page.$('script[data-type="svelte-data"]')).toBeNull();
+	expect(await page.$('script[sveltekit\\:data-type="data"]')).toBeNull();
 });
 
 test('styles are applied', async ({ page, baseURL }) => {

packages/kit/test/apps/basics/test/test.js

@@ -1068,7 +1068,7 @@ test.describe.parallel('Load', () => {
 
 		if (!javaScriptEnabled) {
 			// by the time JS has run, hydration will have nuked these scripts
-			const script_contents = await page.innerHTML('script[data-type="svelte-data"]');
+			const script_contents = await page.innerHTML('script[sveltekit\\:data-type="data"]');
 
 			const payload =
 				'{"status":200,"statusText":"","headers":{"content-type":"application/json; charset=utf-8"},"body":"{\\"answer\\":42}"}';
@@ -1098,11 +1098,11 @@ test.describe.parallel('Load', () => {
 		if (!javaScriptEnabled) {
 			// by the time JS has run, hydration will have nuked these scripts
 			const script_contents_a = await page.innerHTML(
-				'script[data-type="svelte-data"][data-url="/load/serialization-post.json"][data-body="3t25"]'
+				'script[sveltekit\\:data-type="data"][sveltekit\\:data-url="/load/serialization-post.json"][sveltekit\\:data-body="3t25"]'
 			);
 
 			const script_contents_b = await page.innerHTML(
-				'script[data-type="svelte-data"][data-url="/load/serialization-post.json"][data-body="3t24"]'
+				'script[sveltekit\\:data-type="data"][sveltekit\\:data-url="/load/serialization-post.json"][sveltekit\\:data-body="3t24"]'
 			);
 
 			expect(script_contents_a).toBe(payload_a);
@@ -1431,7 +1431,7 @@ test.describe.parallel('Page options', () => {
 			// ensure data wasn't inlined
 			expect(
 				await page.evaluate(
-					() => document.querySelectorAll('script[data-type="svelte-data"]').length
+					() => document.querySelectorAll('script[sveltekit\\:data-type="data"]').length
 				)
 			).toBe(0);
 		}

packages/kit/types/private.d.ts

@@ -241,6 +241,12 @@ export type MaybePromise<T> = T | Promise<T>;
 
 export type Only<T, U> = { [P in keyof T]: T[P] } & { [P in Exclude<keyof U, keyof T>]?: never };
 
+export type PayloadScriptAttributes = PayloadScriptAttributesData | PayloadScriptAttributesProps;
+
+type PayloadScriptAttributesData = { type: 'data'; url: string; body?: string };
+
+type PayloadScriptAttributesProps = { type: 'props' };
+
 export interface Prerendered {
 	pages: Map<
 		string,