From 3b6b5feb48f274f531661d73890bb420d03baccc Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 20 Oct 2025 13:57:54 +0200 Subject: [PATCH 1/8] use $state instead of $state.raw --- .../kit/src/runtime/app/server/remote/form.js | 2 +- .../client/remote-functions/form.svelte.js | 82 ++++++------------- packages/kit/src/runtime/form-utils.svelte.js | 26 +++--- 3 files changed, 34 insertions(+), 76 deletions(-) diff --git a/packages/kit/src/runtime/app/server/remote/form.js b/packages/kit/src/runtime/app/server/remote/form.js index 9d5b5b551a4d..854a8a59ff48 100644 --- a/packages/kit/src/runtime/app/server/remote/form.js +++ b/packages/kit/src/runtime/app/server/remote/form.js @@ -318,7 +318,7 @@ function handle_issues(output, issues, is_remote_request, form_data) { if (is_array) key = key.slice(0, -2); - output.input = set_nested_value( + set_nested_value( /** @type {Record} */ (output.input), key, is_array ? values : values[0] diff --git a/packages/kit/src/runtime/client/remote-functions/form.svelte.js b/packages/kit/src/runtime/client/remote-functions/form.svelte.js index 3352231674c9..8b59c7170154 100644 --- a/packages/kit/src/runtime/client/remote-functions/form.svelte.js +++ b/packages/kit/src/runtime/client/remote-functions/form.svelte.js @@ -17,7 +17,6 @@ import { deep_set, set_nested_value, throw_on_old_property_access, - split_path, build_path_string, normalize_issue } from '../../form-utils.svelte.js'; @@ -60,24 +59,9 @@ export function form(id) { const action = '?/remote=' + encodeURIComponent(action_id); /** - * By making this $state.raw() and creating a new object each time we update it, - * all consumers along the update chain are properly invalidated. * @type {Record} */ - let input = $state.raw({}); - - // TODO 3.0: Remove versions state and related logic; it's a workaround for $derived not updating when created inside $effects - /** - * This allows us to update individual fields granularly - * @type {Record} - */ - const versions = $state({}); - - /** - * This ensures that `{field.value()}` is updated even if the version hasn't been initialized - * @type {Set} - */ - const version_reads = new Set(); + let input = $state({}); /** @type {InternalRemoteFormIssue[]} */ let raw_issues = $state.raw([]); @@ -101,16 +85,6 @@ export function form(id) { let submitted = false; - function update_all_versions() { - for (const path of version_reads) { - versions[path] ??= 0; - } - - for (const key of Object.keys(versions)) { - versions[key] += 1; - } - } - /** * @param {FormData} form_data * @returns {Record} @@ -235,8 +209,6 @@ export function form(id) { if (issues.$) { release_overrides(updates); } else { - update_all_versions(); - if (form_result.refreshes) { refresh_queries(form_result.refreshes, updates); } else { @@ -386,7 +358,7 @@ export function form(id) { } } - input = set_nested_value(input, name, value); + set_nested_value(input, name, value); } else if (is_file) { if (DEV && element.multiple) { throw new Error( @@ -397,7 +369,7 @@ export function form(id) { const file = /** @type {HTMLInputElement & { files: FileList }} */ (element).files[0]; if (file) { - input = set_nested_value(input, name, file); + set_nested_value(input, name, file); } else { // Remove the property by setting to undefined and clean up const path_parts = name.split(/\.|\[|\]/).filter(Boolean); @@ -409,26 +381,18 @@ export function form(id) { delete current[path_parts[path_parts.length - 1]]; } } else { - input = set_nested_value( + console.log('set neseted', input, name, element.value); + set_nested_value( input, name, element.type === 'checkbox' && !element.checked ? null : element.value ); + console.log('afterwards', input); } name = name.replace(/^[nb]:/, ''); - versions[name] ??= 0; - versions[name] += 1; - - const path = split_path(name); - - while (path.pop() !== undefined) { - const name = build_path_string(path); - - versions[name] ??= 0; - versions[name] += 1; - } + touched[name] = true; }); form.addEventListener('reset', async () => { @@ -437,7 +401,14 @@ export function form(id) { await tick(); input = convert_formdata(new FormData(form)); - update_all_versions(); + + // // Clear existing properties + // for (const key in input) { + // delete input[key]; + // } + + // // Copy new properties + // Object.assign(input, new_data); }); return () => { @@ -530,28 +501,21 @@ export function form(id) { create_field_proxy( {}, () => input, - (path) => { - version_reads.add(path); - versions[path]; - }, + () => {}, (path, value) => { if (path.length === 0) { + // // Clear existing properties + // for (const key in input) { + // delete input[key]; + // } + // // Copy new properties + // Object.assign(input, value); input = value; - update_all_versions(); } else { - input = deep_set(input, path.map(String), value); + deep_set(input, path.map(String), value); const key = build_path_string(path); - versions[key] ??= 0; - versions[key] += 1; touched[key] = true; - - const parent_path = path.slice(); - while (parent_path.pop() !== undefined) { - const parent_key = build_path_string(parent_path); - versions[parent_key] ??= 0; - versions[parent_key] += 1; - } } }, () => issues diff --git a/packages/kit/src/runtime/form-utils.svelte.js b/packages/kit/src/runtime/form-utils.svelte.js index e59c1237a83a..625b7cdf5b9b 100644 --- a/packages/kit/src/runtime/form-utils.svelte.js +++ b/packages/kit/src/runtime/form-utils.svelte.js @@ -8,7 +8,7 @@ import * as svelte from 'svelte'; const untrack = svelte.untrack ?? ((value) => value()); /** - * Sets a value in a nested object using a path string, not mutating the original object but returning a new object + * Sets a value in a nested object using a path string, mutating the original object * @param {Record} object * @param {string} path_string * @param {any} value @@ -22,7 +22,7 @@ export function set_nested_value(object, path_string, value) { value = value === 'on'; } - return deep_set(object, split_path(path_string), value); + deep_set(object, split_path(path_string), value); } /** @@ -61,7 +61,7 @@ export function convert_formdata(data) { values = values.map((v) => v === 'on'); } - result = set_nested_value(result, key, is_array ? values : values[0]); + set_nested_value(result, key, is_array ? values : values[0]); } return result; @@ -81,15 +81,13 @@ export function split_path(path) { } /** - * Sets a value in a nested object using an array of keys. - * Does not mutate the original object; returns a new object. + * Sets a value in a nested object using an array of keys, mutating the original object. * @param {Record} object * @param {string[]} keys * @param {any} value */ export function deep_set(object, keys, value) { - const result = Object.assign(Object.create(null), object); // guard against prototype pollution - let current = result; + let current = object; for (let i = 0; i < keys.length - 1; i += 1) { const key = keys[i]; @@ -101,18 +99,14 @@ export function deep_set(object, keys, value) { throw new Error(`Invalid array key ${keys[i + 1]}`); } - current[key] = is_array - ? exists - ? [...inner] - : [] - : // guard against prototype pollution - Object.assign(Object.create(null), inner); + if (!exists) { + current[key] = is_array ? [] : Object.create(null); // guard against prototype pollution + } current = current[key]; } current[keys[keys.length - 1]] = value; - return result; } /** @@ -206,8 +200,8 @@ export function create_field_proxy(target, get_input, depend, set_input, get_iss const path_string = build_path_string(path); const get_value = () => { - depend(path_string); - return untrack(() => deep_get(get_input(), path)); + // depend(path_string); + return deep_get(get_input(), path); }; return new Proxy(target, { From 409bfe14fb4ba20256764d43b6c8d65d88fe5478 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 20 Oct 2025 13:58:21 +0200 Subject: [PATCH 2/8] switch method of prototype pollution prevention (we cannot use Object.create(null) because that isn't proxified by $state) --- packages/kit/src/runtime/form-utils.spec.js | 16 ++++++++++ packages/kit/src/runtime/form-utils.svelte.js | 30 +++++++++++++------ 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/packages/kit/src/runtime/form-utils.spec.js b/packages/kit/src/runtime/form-utils.spec.js index 0e9c263c33b5..3cce421dd3ba 100644 --- a/packages/kit/src/runtime/form-utils.spec.js +++ b/packages/kit/src/runtime/form-utils.spec.js @@ -73,4 +73,20 @@ describe('convert_formdata', () => { } }); }); + + const pollution_attacks = [ + '__proto__.polluted', + 'constructor.polluted', + 'prototype.polluted', + 'user.__proto__.polluted', + 'user.constructor.polluted' + ]; + + for (const attack of pollution_attacks) { + test(`prevents prototype pollution: ${attack}`, () => { + const data = new FormData(); + data.append(attack, 'bad'); + expect(() => convert_formdata(data)).toThrow(/prototype pollution attempt detected/); + }); + } }); diff --git a/packages/kit/src/runtime/form-utils.svelte.js b/packages/kit/src/runtime/form-utils.svelte.js index 625b7cdf5b9b..1b86a3168133 100644 --- a/packages/kit/src/runtime/form-utils.svelte.js +++ b/packages/kit/src/runtime/form-utils.svelte.js @@ -3,9 +3,6 @@ /** @import { StandardSchemaV1 } from '@standard-schema/spec' */ import { DEV } from 'esm-env'; -import * as svelte from 'svelte'; -// Svelte 4 and under don't have `untrack` - you'll not be able to use remote functions with Svelte 4 but this will still be loaded -const untrack = svelte.untrack ?? ((value) => value()); /** * Sets a value in a nested object using a path string, mutating the original object @@ -31,7 +28,7 @@ export function set_nested_value(object, path_string, value) { */ export function convert_formdata(data) { /** @type {Record} */ - let result = Object.create(null); // guard against prototype pollution + let result = {}; for (let key of data.keys()) { if (key.startsWith('sveltekit:')) { @@ -80,6 +77,19 @@ export function split_path(path) { return path.split(/\.|\[|\]/).filter(Boolean); } +/** + * Check if a property key is dangerous and could lead to prototype pollution + * @param {string} key + */ +function check_prototype_pollution(key) { + if (key === '__proto__' || key === 'constructor' || key === 'prototype') { + throw new Error( + `Invalid key "${key}"` + + (DEV ? ': This key is not allowed to prevent prototype pollution.' : '') + ); + } +} + /** * Sets a value in a nested object using an array of keys, mutating the original object. * @param {Record} object @@ -91,6 +101,9 @@ export function deep_set(object, keys, value) { for (let i = 0; i < keys.length - 1; i += 1) { const key = keys[i]; + + check_prototype_pollution(key); + const is_array = /^\d+$/.test(keys[i + 1]); const exists = key in current; const inner = current[key]; @@ -100,13 +113,15 @@ export function deep_set(object, keys, value) { } if (!exists) { - current[key] = is_array ? [] : Object.create(null); // guard against prototype pollution + current[key] = is_array ? [] : {}; } current = current[key]; } - current[keys[keys.length - 1]] = value; + const final_key = keys[keys.length - 1]; + check_prototype_pollution(final_key); + current[final_key] = value; } /** @@ -197,10 +212,7 @@ export function deep_get(object, path) { * @returns {any} Proxy object with name(), value(), and issues() methods */ export function create_field_proxy(target, get_input, depend, set_input, get_issues, path = []) { - const path_string = build_path_string(path); - const get_value = () => { - // depend(path_string); return deep_get(get_input(), path); }; From 7aa7716315d653c89958d597bc6c318f581650c0 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 20 Oct 2025 14:07:00 +0200 Subject: [PATCH 3/8] cleanup --- .../client/remote-functions/form.svelte.js | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/packages/kit/src/runtime/client/remote-functions/form.svelte.js b/packages/kit/src/runtime/client/remote-functions/form.svelte.js index 8b59c7170154..aaa9624f8781 100644 --- a/packages/kit/src/runtime/client/remote-functions/form.svelte.js +++ b/packages/kit/src/runtime/client/remote-functions/form.svelte.js @@ -381,13 +381,11 @@ export function form(id) { delete current[path_parts[path_parts.length - 1]]; } } else { - console.log('set neseted', input, name, element.value); set_nested_value( input, name, element.type === 'checkbox' && !element.checked ? null : element.value ); - console.log('afterwards', input); } name = name.replace(/^[nb]:/, ''); @@ -401,14 +399,6 @@ export function form(id) { await tick(); input = convert_formdata(new FormData(form)); - - // // Clear existing properties - // for (const key in input) { - // delete input[key]; - // } - - // // Copy new properties - // Object.assign(input, new_data); }); return () => { @@ -504,12 +494,6 @@ export function form(id) { () => {}, (path, value) => { if (path.length === 0) { - // // Clear existing properties - // for (const key in input) { - // delete input[key]; - // } - // // Copy new properties - // Object.assign(input, value); input = value; } else { deep_set(input, path.map(String), value); From 0b33df2acf46b512bc8189f071c80bcc0074d22a Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 20 Oct 2025 14:11:45 +0200 Subject: [PATCH 4/8] changeset --- .changeset/young-pans-matter.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/young-pans-matter.md diff --git a/.changeset/young-pans-matter.md b/.changeset/young-pans-matter.md new file mode 100644 index 000000000000..e9476f7dc890 --- /dev/null +++ b/.changeset/young-pans-matter.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +fix: rework internal representation of form value to be `$state` From 341509a6d031a9f632a73b5453394767811119d7 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 20 Oct 2025 14:12:51 +0200 Subject: [PATCH 5/8] obsolete --- .../kit/src/runtime/app/server/remote/form.js | 1 - .../client/remote-functions/form.svelte.js | 1 - packages/kit/src/runtime/form-utils.svelte.js | 27 +++++-------------- 3 files changed, 7 insertions(+), 22 deletions(-) diff --git a/packages/kit/src/runtime/app/server/remote/form.js b/packages/kit/src/runtime/app/server/remote/form.js index 854a8a59ff48..61bfa3f4bce5 100644 --- a/packages/kit/src/runtime/app/server/remote/form.js +++ b/packages/kit/src/runtime/app/server/remote/form.js @@ -215,7 +215,6 @@ export function form(validate_or_fn, maybe_fn) { return create_field_proxy( {}, () => data?.input ?? {}, - () => {}, (path, value) => { if (data?.submission) { // don't override a submission diff --git a/packages/kit/src/runtime/client/remote-functions/form.svelte.js b/packages/kit/src/runtime/client/remote-functions/form.svelte.js index aaa9624f8781..42359c5636a8 100644 --- a/packages/kit/src/runtime/client/remote-functions/form.svelte.js +++ b/packages/kit/src/runtime/client/remote-functions/form.svelte.js @@ -491,7 +491,6 @@ export function form(id) { create_field_proxy( {}, () => input, - () => {}, (path, value) => { if (path.length === 0) { input = value; diff --git a/packages/kit/src/runtime/form-utils.svelte.js b/packages/kit/src/runtime/form-utils.svelte.js index 1b86a3168133..f57a63867df7 100644 --- a/packages/kit/src/runtime/form-utils.svelte.js +++ b/packages/kit/src/runtime/form-utils.svelte.js @@ -205,13 +205,12 @@ export function deep_get(object, path) { * Creates a proxy-based field accessor for form data * @param {any} target - Function or empty POJO * @param {() => Record} get_input - Function to get current input data - * @param {(path: string) => void} depend - Function to make an effect depend on a specific field * @param {(path: (string | number)[], value: any) => void} set_input - Function to set input data * @param {() => Record} get_issues - Function to get current issues * @param {(string | number)[]} path - Current access path * @returns {any} Proxy object with name(), value(), and issues() methods */ -export function create_field_proxy(target, get_input, depend, set_input, get_issues, path = []) { +export function create_field_proxy(target, get_input, set_input, get_issues, path = []) { const get_value = () => { return deep_get(get_input(), path); }; @@ -222,7 +221,7 @@ export function create_field_proxy(target, get_input, depend, set_input, get_iss // Handle array access like jobs[0] if (/^\d+$/.test(prop)) { - return create_field_proxy({}, get_input, depend, set_input, get_issues, [ + return create_field_proxy({}, get_input, set_input, get_issues, [ ...path, parseInt(prop, 10) ]); @@ -235,17 +234,11 @@ export function create_field_proxy(target, get_input, depend, set_input, get_iss set_input(path, newValue); return newValue; }; - return create_field_proxy(set_func, get_input, depend, set_input, get_issues, [ - ...path, - prop - ]); + return create_field_proxy(set_func, get_input, set_input, get_issues, [...path, prop]); } if (prop === 'value') { - return create_field_proxy(get_value, get_input, depend, set_input, get_issues, [ - ...path, - prop - ]); + return create_field_proxy(get_value, get_input, set_input, get_issues, [...path, prop]); } if (prop === 'issues' || prop === 'allIssues') { @@ -265,10 +258,7 @@ export function create_field_proxy(target, get_input, depend, set_input, get_iss })); }; - return create_field_proxy(issues_func, get_input, depend, set_input, get_issues, [ - ...path, - prop - ]); + return create_field_proxy(issues_func, get_input, set_input, get_issues, [...path, prop]); } if (prop === 'as') { @@ -417,14 +407,11 @@ export function create_field_proxy(target, get_input, depend, set_input, get_iss }); }; - return create_field_proxy(as_func, get_input, depend, set_input, get_issues, [ - ...path, - 'as' - ]); + return create_field_proxy(as_func, get_input, set_input, get_issues, [...path, 'as']); } // Handle property access (nested fields) - return create_field_proxy({}, get_input, depend, set_input, get_issues, [...path, prop]); + return create_field_proxy({}, get_input, set_input, get_issues, [...path, prop]); } }); } From 312f5b34905ec30c2acc08ac879dd21932aff00c Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 20 Oct 2025 15:48:57 +0200 Subject: [PATCH 6/8] oops --- packages/kit/src/runtime/form-utils.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/form-utils.spec.js b/packages/kit/src/runtime/form-utils.spec.js index 3cce421dd3ba..12ea00fb4a9a 100644 --- a/packages/kit/src/runtime/form-utils.spec.js +++ b/packages/kit/src/runtime/form-utils.spec.js @@ -86,7 +86,7 @@ describe('convert_formdata', () => { test(`prevents prototype pollution: ${attack}`, () => { const data = new FormData(); data.append(attack, 'bad'); - expect(() => convert_formdata(data)).toThrow(/prototype pollution attempt detected/); + expect(() => convert_formdata(data)).toThrow(/Invalid key "/); }); } }); From f833a6c61d24801831e8971893c229c91fedc683 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 20 Oct 2025 16:00:03 +0200 Subject: [PATCH 7/8] lint --- packages/kit/src/runtime/form-utils.svelte.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/form-utils.svelte.js b/packages/kit/src/runtime/form-utils.svelte.js index f57a63867df7..e268ff80b2b8 100644 --- a/packages/kit/src/runtime/form-utils.svelte.js +++ b/packages/kit/src/runtime/form-utils.svelte.js @@ -28,7 +28,7 @@ export function set_nested_value(object, path_string, value) { */ export function convert_formdata(data) { /** @type {Record} */ - let result = {}; + const result = {}; for (let key of data.keys()) { if (key.startsWith('sveltekit:')) { From 41cd6365cdbc2c790677e17c9fbf5d6eae3aab0b Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 20 Oct 2025 16:00:37 +0200 Subject: [PATCH 8/8] doesnt need to be a .svelte.js anymore --- packages/kit/src/runtime/app/server/remote/form.js | 2 +- packages/kit/src/runtime/client/remote-functions/form.svelte.js | 2 +- .../kit/src/runtime/{form-utils.svelte.js => form-utils.js} | 0 packages/kit/src/runtime/form-utils.spec.js | 2 +- 4 files changed, 3 insertions(+), 3 deletions(-) rename packages/kit/src/runtime/{form-utils.svelte.js => form-utils.js} (100%) diff --git a/packages/kit/src/runtime/app/server/remote/form.js b/packages/kit/src/runtime/app/server/remote/form.js index 61bfa3f4bce5..7bc7259185e9 100644 --- a/packages/kit/src/runtime/app/server/remote/form.js +++ b/packages/kit/src/runtime/app/server/remote/form.js @@ -11,7 +11,7 @@ import { deep_set, normalize_issue, flatten_issues -} from '../../../form-utils.svelte.js'; +} from '../../../form-utils.js'; import { get_cache, run_remote_function } from './shared.js'; /** diff --git a/packages/kit/src/runtime/client/remote-functions/form.svelte.js b/packages/kit/src/runtime/client/remote-functions/form.svelte.js index 42359c5636a8..02bbf9239720 100644 --- a/packages/kit/src/runtime/client/remote-functions/form.svelte.js +++ b/packages/kit/src/runtime/client/remote-functions/form.svelte.js @@ -19,7 +19,7 @@ import { throw_on_old_property_access, build_path_string, normalize_issue -} from '../../form-utils.svelte.js'; +} from '../../form-utils.js'; /** * Merge client issues into server issues. Server issues are persisted unless diff --git a/packages/kit/src/runtime/form-utils.svelte.js b/packages/kit/src/runtime/form-utils.js similarity index 100% rename from packages/kit/src/runtime/form-utils.svelte.js rename to packages/kit/src/runtime/form-utils.js diff --git a/packages/kit/src/runtime/form-utils.spec.js b/packages/kit/src/runtime/form-utils.spec.js index 12ea00fb4a9a..77b7e355f4a8 100644 --- a/packages/kit/src/runtime/form-utils.spec.js +++ b/packages/kit/src/runtime/form-utils.spec.js @@ -1,5 +1,5 @@ import { describe, expect, test } from 'vitest'; -import { convert_formdata, split_path } from './form-utils.svelte.js'; +import { convert_formdata, split_path } from './form-utils.js'; describe('split_path', () => { const good = [