Skip to content

Commit d3300c6

Browse files
authored
Merge commit from fork
* fix: escape names of tracked search parameters * oops
1 parent e39d2e7 commit d3300c6

File tree

7 files changed

+43
-16
lines changed

7 files changed

+43
-16
lines changed

.changeset/rotten-colts-arrive.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@sveltejs/kit': patch
3+
---
4+
5+
fix: escape names of tracked search parameters

packages/kit/src/runtime/server/data/index.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { HttpError, SvelteKitError, Redirect } from '../../control.js';
22
import { normalize_error } from '../../../utils/error.js';
33
import { once } from '../../../utils/functions.js';
44
import { load_server_data } from '../page/load_data.js';
5-
import { clarify_devalue_error, handle_error_and_jsonify, stringify_uses } from '../utils.js';
5+
import { clarify_devalue_error, handle_error_and_jsonify, serialize_uses } from '../utils.js';
66
import { normalize_path } from '../../../utils/url.js';
77
import { text } from '../../../exports/index.js';
88
import * as devalue from 'devalue';
@@ -253,8 +253,8 @@ export function get_data_json(event, options, nodes) {
253253
return JSON.stringify(node);
254254
}
255255

256-
return `{"type":"data","data":${devalue.stringify(node.data, reducers)},${stringify_uses(
257-
node
256+
return `{"type":"data","data":${devalue.stringify(node.data, reducers)},${JSON.stringify(
257+
serialize_uses(node)
258258
)}${node.slash ? `,"slash":${JSON.stringify(node.slash)}` : ''}}`;
259259
});
260260

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

+6-4
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { serialize_data } from './serialize_data.js';
77
import { s } from '../../../utils/misc.js';
88
import { Csp } from './csp.js';
99
import { uneval_action_response } from './actions.js';
10-
import { clarify_devalue_error, stringify_uses, handle_error_and_jsonify } from '../utils.js';
10+
import { clarify_devalue_error, handle_error_and_jsonify, serialize_uses } from '../utils.js';
1111
import { public_env, safe_public_env } from '../../shared-server.js';
1212
import { text } from '../../../exports/index.js';
1313
import { create_async_iterator } from '../../../utils/streaming.js';
@@ -646,9 +646,11 @@ function get_data(event, options, nodes, csp, global) {
646646
const strings = nodes.map((node) => {
647647
if (!node) return 'null';
648648

649-
return `{"type":"data","data":${devalue.uneval(node.data, replacer)},${stringify_uses(node)}${
650-
node.slash ? `,"slash":${JSON.stringify(node.slash)}` : ''
651-
}}`;
649+
/** @type {any} */
650+
const payload = { type: 'data', data: node.data, uses: serialize_uses(node) };
651+
if (node.slash) payload.slash = node.slash;
652+
653+
return devalue.uneval(payload, replacer);
652654
});
653655

654656
return {

packages/kit/src/runtime/server/utils.js

+9-9
Original file line numberDiff line numberDiff line change
@@ -147,26 +147,26 @@ export function clarify_devalue_error(event, error) {
147147
/**
148148
* @param {import('types').ServerDataNode} node
149149
*/
150-
export function stringify_uses(node) {
151-
const uses = [];
150+
export function serialize_uses(node) {
151+
const uses = {};
152152

153153
if (node.uses && node.uses.dependencies.size > 0) {
154-
uses.push(`"dependencies":${JSON.stringify(Array.from(node.uses.dependencies))}`);
154+
uses.dependencies = Array.from(node.uses.dependencies);
155155
}
156156

157157
if (node.uses && node.uses.search_params.size > 0) {
158-
uses.push(`"search_params":${JSON.stringify(Array.from(node.uses.search_params))}`);
158+
uses.search_params = Array.from(node.uses.search_params);
159159
}
160160

161161
if (node.uses && node.uses.params.size > 0) {
162-
uses.push(`"params":${JSON.stringify(Array.from(node.uses.params))}`);
162+
uses.params = Array.from(node.uses.params);
163163
}
164164

165-
if (node.uses?.parent) uses.push('"parent":1');
166-
if (node.uses?.route) uses.push('"route":1');
167-
if (node.uses?.url) uses.push('"url":1');
165+
if (node.uses?.parent) uses.parent = 1;
166+
if (node.uses?.route) uses.route = 1;
167+
if (node.uses?.url) uses.url = 1;
168168

169-
return `"uses":{${uses.join(',')}}`;
169+
return uses;
170170
}
171171

172172
/**
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
export function load({ url }) {
2+
const values = {};
3+
4+
for (const key of url.searchParams.keys()) {
5+
values[key] = url.searchParams.get(key);
6+
}
7+
8+
return {
9+
values
10+
};
11+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
<p>check window.pwned</p>

packages/kit/test/apps/basics/test/cross-platform/test.js

+8
Original file line numberDiff line numberDiff line change
@@ -1049,6 +1049,14 @@ test.describe('XSS', () => {
10491049
'user.name is </script><script>window.pwned = 1</script>'
10501050
);
10511051
});
1052+
1053+
test('no xss via tracked search parameters', async ({ page }) => {
1054+
// https://github.com/sveltejs/kit/security/advisories/GHSA-6q87-84jw-cjhp
1055+
await page.goto('/xss/query-tracking?</script/><script>window.pwned%3D1</script/>');
1056+
1057+
// @ts-expect-error - check global injected variable
1058+
expect(await page.evaluate(() => window.pwned)).toBeUndefined();
1059+
});
10521060
});
10531061

10541062
test.describe('$app/server', () => {

0 commit comments

Comments
 (0)