diff --git a/src/backend/drivers/kv/KVStoreDriver.test.ts b/src/backend/drivers/kv/KVStoreDriver.test.ts index 59f4e2cf8..807c70d7f 100644 --- a/src/backend/drivers/kv/KVStoreDriver.test.ts +++ b/src/backend/drivers/kv/KVStoreDriver.test.ts @@ -463,6 +463,23 @@ describe('KVStoreDriver', () => { ), ).rejects.toMatchObject({ statusCode: 400 }); }); + + // A path walking the prototype chain is a client error, not an + // opaque 500 out of the document client. + it.each([ + ['incr' as const], + ['decr' as const], + ])('%s rejects a prototype-walking path as a 400', async (op) => { + await expect( + inCtx(() => + target[op]({ + key: 'proto-test', + pathAndAmountMap: { 'constructor.prototype.x': 1 }, + }), + ), + ).rejects.toMatchObject({ statusCode: 400 }); + expect(({} as Record).x).toBeUndefined(); + }); }); describe('expireAt / expire', () => { diff --git a/src/backend/services/auth/TokenService.test.ts b/src/backend/services/auth/TokenService.test.ts index 0b314762f..be3329293 100644 --- a/src/backend/services/auth/TokenService.test.ts +++ b/src/backend/services/auth/TokenService.test.ts @@ -17,6 +17,7 @@ * along with this program. If not, see . */ +import { createHmac } from 'node:crypto'; import jwt from 'jsonwebtoken'; import { describe, expect, it } from 'vitest'; import { TokenService } from './TokenService.js'; @@ -367,3 +368,54 @@ describe('TokenService.verify — algorithm pinning', () => { expect(() => svc.verify('auth', token)).toThrow(/algorithm/); }); }); + +describe('TokenService payload key handling', () => { + // Decompression looks every field name up in a table keyed by claim + // name. A crafted token can name a field after an `Object.prototype` + // member, which must be read as data rather than as a field definition. + const prototypeKeys = ['constructor', '__proto__', 'toString']; + + /** + * Hand-mint a v2-shaped token. `jwt.sign` refuses these payloads (its own + * claim validator has the same prototype-lookup flaw), which is exactly + * why an attacker assembles the JWT by hand instead. + */ + const mintRawV2Token = (payloadJson: string, sign = true): string => { + const b64 = (s: string) => Buffer.from(s).toString('base64url'); + const head = b64( + JSON.stringify({ alg: 'HS256', typ: 'JWT', kid: 'v2' }), + ); + const body = b64(payloadJson); + const sig = sign + ? createHmac('sha256', V2_SECRET) + .update(`${head}.${body}`) + .digest('base64url') + : 'not-a-signature'; + return `${head}.${body}.${sig}`; + }; + + it.each(prototypeKeys)( + 'decodes an unsigned payload carrying a `%s` field', + (key) => { + const svc = createTokenService(); + const token = mintRawV2Token(`{"t":"s","${key}":"x"}`, false); + const decoded = svc.decodeWithoutVerify>( + 'auth', + token, + ); + expect(decoded).toMatchObject({ type: 'session' }); + expect(Object.getPrototypeOf(decoded)).toBe(Object.prototype); + }, + ); + + it.each(prototypeKeys)( + 'verifies a signed payload carrying a `%s` field', + (key) => { + const svc = createTokenService(); + const token = mintRawV2Token(`{"t":"s","${key}":"x"}`); + const verified = svc.verify>('auth', token); + expect(verified).toMatchObject({ type: 'session' }); + expect(Object.getPrototypeOf(verified)).toBe(Object.prototype); + }, + ); +}); diff --git a/src/backend/services/auth/TokenService.ts b/src/backend/services/auth/TokenService.ts index 913afe7a2..faabba238 100644 --- a/src/backend/services/auth/TokenService.ts +++ b/src/backend/services/auth/TokenService.ts @@ -3,18 +3,19 @@ * * This file is part of Puter. * - * Puter is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published - * by the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. + * Puter is free software: you can redistribute it and/or modify it under the + * terms of the GNU Affero General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) any + * later version. * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more + * details. * * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . + * along with this program. If not, see + * [https://www.gnu.org/licenses/](https://www.gnu.org/licenses/). */ import jwt, { type SignOptions } from 'jsonwebtoken'; @@ -52,9 +53,32 @@ interface CompressionContext { short_to_fullkey: Record; } +/** + * Table lookups keyed by a JWT field name. A decoded payload is attacker- + * shaped (`decodeWithoutVerify` runs before any signature check), and a literal + * `constructor` or `__proto__` key would otherwise resolve to an + * `Object.prototype` member and be mistaken for a field definition. + */ +const lookupOwn = (table: Record, key: string): T | undefined => + Object.hasOwn(table, key) ? table[key] : undefined; + +/** Write a key as data, so a `__proto__` field can't reassign the prototype. */ +const setOwn = ( + target: Record, + key: string, + value: unknown, +): void => { + Object.defineProperty(target, key, { + value, + writable: true, + enumerable: true, + configurable: true, + }); +}; + const def = (o: Record): CompressionContext => { const fullkey_to_info: Record = {}; - for (const k in o) { + for (const k of Object.keys(o)) { const v = o[k]; fullkey_to_info[k] = typeof v === 'string' ? { short: v } : v; } @@ -165,10 +189,10 @@ const COMPRESSION: Record = { /** * Thrown by `verify()` when a v1 token is presented while - * `auth.allow_v1_tokens=false`. Carries the **unverified** payload so - * the auth probe can mint a `reauth_required` response with an - * `auth_id` hint — the hint is advisory only (never trusted as - * identity), so reading it from an unsigned payload is safe. + * `auth.allow_v1_tokens=false`. Carries the **unverified** payload so the auth + * probe can mint a `reauth_required` response with an `auth_id` hint — the hint + * is advisory only (never trusted as identity), so reading it from an unsigned + * payload is safe. */ export class V1TokensDisabledError extends Error { constructor(public readonly payload: Record) { @@ -270,9 +294,10 @@ export class TokenService extends PuterService { /** * Verify and decompress. Routes by header `kid`: - * - `kid === 'v2'` → verify against the v2 secret. - * - else → verify against the legacy secret and tag the result with - * `legacy: true`. Rejected outright if `allow_v1_tokens=false`. + * + * - `kid === 'v2'` → verify against the v2 secret. + * - Else → verify against the legacy secret and tag the result with `legacy: + * true`. Rejected outright if `allow_v1_tokens=false`. * * Throws on invalid signature / expired / malformed (propagates * `jsonwebtoken`'s errors). Callers in the auth probe should catch and @@ -332,14 +357,13 @@ export class TokenService extends PuterService { } /** - * Decode + decompress *without* verifying the signature. Returns - * `null` for malformed tokens. Use **only** for paths that need to - * recover advisory hints from an expired / unsignable token (e.g., - * the logout path that wants to revoke a session row even if the - * JWT has expired since the user opened the tab). The result is - * never trusted as identity — only as a pointer for cleanup - * operations the caller would otherwise authorize via a different - * channel. + * Decode + decompress _without_ verifying the signature. Returns `null` for + * malformed tokens. Use **only** for paths that need to recover advisory + * hints from an expired / unsignable token (e.g., the logout path that + * wants to revoke a session row even if the JWT has expired since the user + * opened the tab). The result is never trusted as identity — only as a + * pointer for cleanup operations the caller would otherwise authorize via a + * different channel. */ decodeWithoutVerify>( scope: string, @@ -363,10 +387,10 @@ export class TokenService extends PuterService { if (!context) return payload; const { fullkey_to_info } = context; const out: Record = {}; - for (const fullkey in payload) { - const info = fullkey_to_info[fullkey]; + for (const fullkey of Object.keys(payload)) { + const info = lookupOwn(fullkey_to_info, fullkey); if (!info) { - out[fullkey] = payload[fullkey]; + setOwn(out, fullkey, payload[fullkey]); continue; } let k = fullkey; @@ -375,13 +399,13 @@ export class TokenService extends PuterService { if ( info.values && typeof v === 'string' && - info.values.to_short[v] !== undefined + lookupOwn(info.values.to_short, v) !== undefined ) { v = info.values.to_short[v]; } else if (info.encode && typeof v === 'string') { v = info.encode(v); } - out[k] = v; + setOwn(out, k, v); } return out; } @@ -393,26 +417,28 @@ export class TokenService extends PuterService { if (!context) return payload; const { fullkey_to_info, short_to_fullkey } = context; const out: Record = {}; - for (const short in payload) { - const fullkey = short_to_fullkey[short]; - if (!fullkey) { - out[short] = payload[short]; + for (const short of Object.keys(payload)) { + const fullkey = lookupOwn(short_to_fullkey, short); + const info = fullkey + ? lookupOwn(fullkey_to_info, fullkey) + : undefined; + if (!fullkey || !info) { + setOwn(out, short, payload[short]); continue; } - const info = fullkey_to_info[fullkey]; let k = short; let v = payload[short]; if (info.short) k = fullkey; if ( info.values && typeof v === 'string' && - info.values.to_long[v] !== undefined + lookupOwn(info.values.to_long, v) !== undefined ) { v = info.values.to_long[v]; } else if (info.decode && typeof v === 'string') { v = info.decode(v); } - out[k] = v; + setOwn(out, k, v); } return out; } diff --git a/src/backend/stores/systemKv/SystemKVStore.test.ts b/src/backend/stores/systemKv/SystemKVStore.test.ts index 0e95c65e9..876684103 100644 --- a/src/backend/stores/systemKv/SystemKVStore.test.ts +++ b/src/backend/stores/systemKv/SystemKVStore.test.ts @@ -1,4 +1,12 @@ -import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'; +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + it, +} from 'vitest'; import { setupTestServer } from '../../testUtil.ts'; import type { SystemKVStore } from './SystemKVStore.ts'; import { PuterServer } from '../../server.ts'; @@ -713,6 +721,153 @@ describe('SystemKVStore', () => { }); }); + describe('prototype-chain safety', () => { + const unsafeSegments = ['__proto__', 'constructor', 'prototype']; + + afterEach(() => { + // Nothing a caller sends may end up on the shared prototype. + expect( + Object.getOwnPropertyNames(Object.prototype), + ).not.toContain('polluted'); + expect(({} as Record).polluted).toBeUndefined(); + }); + + it.each(unsafeSegments)( + 'incr rejects a path traversing `%s`', + async (segment) => { + await expect( + target.incr( + { + key: 'proto-incr', + pathAndAmountMap: { + [`${segment}.polluted.deep`]: 1, + }, + }, + opts, + ), + ).rejects.toMatchObject({ statusCode: 400 }); + }, + ); + + it.each(unsafeSegments)( + 'decr rejects a path traversing `%s`', + async (segment) => { + await expect( + target.decr( + { + key: 'proto-decr', + pathAndAmountMap: { + [`${segment}.polluted.deep`]: 1, + }, + }, + opts, + ), + ).rejects.toMatchObject({ statusCode: 400 }); + }, + ); + + it.each(unsafeSegments)( + 'update rejects a path traversing `%s`', + async (segment) => { + await expect( + target.update( + { + key: 'proto-update', + pathAndValueMap: { + [`${segment}.polluted.deep`]: 1, + }, + }, + opts, + ), + ).rejects.toMatchObject({ statusCode: 400 }); + }, + ); + + it.each(unsafeSegments)( + 'add rejects a path traversing `%s`', + async (segment) => { + await expect( + target.add( + { + key: 'proto-add', + pathAndValueMap: { + [`${segment}.polluted.deep`]: 1, + }, + }, + opts, + ), + ).rejects.toMatchObject({ statusCode: 400 }); + }, + ); + + it.each(unsafeSegments)( + 'remove rejects a path traversing `%s`', + async (segment) => { + await expect( + target.remove( + { key: 'proto-remove', paths: [`${segment}.polluted`] }, + opts, + ), + ).rejects.toMatchObject({ statusCode: 400 }); + }, + ); + + it('rejects an unsafe segment anywhere in the path', async () => { + await expect( + target.incr( + { + key: 'proto-deep', + pathAndAmountMap: { 'a.b.__proto__.polluted': 1 }, + }, + opts, + ), + ).rejects.toMatchObject({ statusCode: 400 }); + }); + + it.each(unsafeSegments)( + 'set rejects a value carrying a `%s` key', + async (unsafeKey) => { + // A JSON body can carry these as real own properties even + // though an object literal in JS source cannot. + const value = JSON.parse(`{"${unsafeKey}":{"polluted":true}}`); + await expect( + target.set({ key: 'proto-set', value }, opts), + ).rejects.toMatchObject({ statusCode: 400 }); + }, + ); + + it('rejects an unsafe key nested deep inside a value', async () => { + const value = JSON.parse( + '{"a":[{"b":{"constructor":{"polluted":true}}}]}', + ); + await expect( + target.set({ key: 'proto-nested', value }, opts), + ).rejects.toMatchObject({ statusCode: 400 }); + }); + + it('rejects an unsafe key in a batchPut item', async () => { + const value = JSON.parse('{"__proto__":{"polluted":true}}'); + await expect( + target.batchPut({ items: [{ key: 'proto-batch', value }] }, opts), + ).rejects.toMatchObject({ statusCode: 400 }); + }); + + it('still allows ordinary paths and values', async () => { + await target.set( + { key: 'proto-ok', value: { safe: { nested: true } } }, + opts, + ); + const stored = await target.get({ key: 'proto-ok' }, opts); + expect(stored.res).toEqual({ safe: { nested: true } }); + + const bumped = await target.incr( + { key: 'proto-ok-counter', pathAndAmountMap: { 'a.b': 2 } }, + opts, + ); + expect(bumped.res).toMatchObject({ a: { b: 2 } }); + }); + }); + describe('usage accounting', () => { it('reports write usage on set and read usage on get', async () => { const setRes = await target.set( diff --git a/src/backend/stores/systemKv/SystemKVStore.ts b/src/backend/stores/systemKv/SystemKVStore.ts index a14a3c0dd..06aa6877a 100644 --- a/src/backend/stores/systemKv/SystemKVStore.ts +++ b/src/backend/stores/systemKv/SystemKVStore.ts @@ -3,18 +3,19 @@ * * This file is part of Puter. * - * Puter is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published - * by the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. + * Puter is free software: you can redistribute it and/or modify it under the + * terms of the GNU Affero General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) any + * later version. * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more + * details. * * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . + * along with this program. If not, see + * [https://www.gnu.org/licenses/](https://www.gnu.org/licenses/). */ import { PuterStore } from '../types'; @@ -41,8 +42,10 @@ export interface KVUsage { write: number; } -/** Standard return envelope: `res` is the operation result, `usage` is the - * DynamoDB consumed capacity so callers can meter if they choose to. */ +/** + * Standard return envelope: `res` is the operation result, `usage` is the + * DynamoDB consumed capacity so callers can meter if they choose to. + */ export interface KVResult { res: T; usage: KVUsage; @@ -117,7 +120,73 @@ const assertKey = (key: string): void => { } }; -const assertValueSize = (value: unknown): void => { +/** + * Object keys we refuse to store or to walk, anywhere in a value or a document + * path. + * + * `__proto__` is the prototype-pollution vector: any code that walks a + * caller-supplied path (`createPaths` below, and every future traversal) would + * step onto `Object.prototype` and write there. It also can't round-trip — the + * AWS document client unmarshalls maps with plain assignment, so a stored + * `__proto__` attribute comes back as the object's prototype instead of as + * data. `constructor`/`prototype` are the second hop of the same walk, and a + * map holding a `constructor` key fails the document client's own "is this a + * plain object" test, which surfaces as an opaque 500 rather than a client + * error. + */ +const UNSAFE_OBJECT_KEYS: ReadonlySet = new Set([ + '__proto__', + 'constructor', + 'prototype', +]); + +const unsafeKeyError = (key: string, subject: string): HttpError => + new HttpError(400, `kv: ${subject} \`${key}\` is not allowed`, { + legacyCode: 'bad_request', + }); + +/** + * Reject a caller-supplied document path (`a.b.c`, optionally with `[0]` list + * indexes) whose segments would walk onto the prototype chain. + */ +const assertPath = (valPath: string): void => { + if (typeof valPath !== 'string') + throw new HttpError(400, 'kv: path must be a string', { + legacyCode: 'bad_request', + }); + for (const chunk of valPath.split('.')) { + const name = chunk.split(/\[\d*\]/g)[0]; + if (UNSAFE_OBJECT_KEYS.has(name)) + throw unsafeKeyError(name, 'path segment'); + } +}; + +const assertPaths = (paths: string[]): void => { + for (const valPath of paths) assertPath(valPath); +}; + +/** + * Walk a value about to be stored and reject unsafe keys. Iterative so a deeply + * nested value can't blow the stack; own keys only, matching what the document + * client actually marshalls. + */ +const assertSafeValueKeys = (value: unknown): void => { + const stack: unknown[] = [value]; + while (stack.length > 0) { + const current = stack.pop(); + if (!current || typeof current !== 'object') continue; + if (Array.isArray(current)) { + stack.push(...current); + continue; + } + for (const [k, v] of Object.entries(current)) { + if (UNSAFE_OBJECT_KEYS.has(k)) throw unsafeKeyError(k, 'value key'); + stack.push(v); + } + } +}; + +const assertValue = (value: unknown): void => { const size = Buffer.byteLength(JSON.stringify(value ?? null), 'utf8'); if (size > MAX_VALUE_BYTES) { throw new HttpError( @@ -126,6 +195,7 @@ const assertValueSize = (value: unknown): void => { { legacyCode: 'bad_request' }, ); } + assertSafeValueKeys(value); }; const normalizePattern = (pattern?: string): string | undefined => { @@ -170,8 +240,8 @@ const cleanAttrName = (chunk: string): string => * * Every method returns `{ res, usage }` — `res` is the operation result, * `usage` is the DynamoDB consumed capacity split into read/write units so - * callers can meter at the driver level when needed. No metering happens - * inside the store itself. + * callers can meter at the driver level when needed. No metering happens inside + * the store itself. * * If `opts.actor` is omitted, operations are scoped to the system namespace. */ @@ -252,7 +322,7 @@ export class SystemKVStore extends PuterStore { opts?: KVOpts, ): Promise> { assertKey(key); - assertValueSize(value); + assertValue(value); const actor = ensureActor(opts); const namespace = getNamespace(actor, opts?.appUuid); @@ -288,7 +358,7 @@ export class SystemKVStore extends PuterStore { for (const item of items) { const k = String(item.key); assertKey(k); - assertValueSize(item.value); + assertValue(item.value); byKey.set(k, { key: k, value: item.value, @@ -619,6 +689,7 @@ export class SystemKVStore extends PuterStore { { legacyCode: 'bad_request' }, ); } + assertPaths(Object.keys(pathAndAmountMap)); const actor = ensureActor(opts); const namespace = getNamespace(actor, opts?.appUuid); @@ -732,8 +803,9 @@ export class SystemKVStore extends PuterStore { }); } for (const val of Object.values(pathAndValueMap)) { - assertValueSize(val); + assertValue(val); } + assertPaths(Object.keys(pathAndValueMap)); const actor = ensureActor(opts); const namespace = getNamespace(actor, opts?.appUuid); @@ -800,6 +872,7 @@ export class SystemKVStore extends PuterStore { legacyCode: 'bad_request', }); } + assertPaths(paths); const actor = ensureActor(opts); const namespace = getNamespace(actor, opts?.appUuid); @@ -878,8 +951,9 @@ export class SystemKVStore extends PuterStore { }); } for (const val of Object.values(pathAndValueMap)) { - assertValueSize(val); + assertValue(val); } + assertPaths(Object.keys(pathAndValueMap)); const actor = ensureActor(opts); const namespace = getNamespace(actor, opts?.appUuid); @@ -1021,14 +1095,16 @@ export class SystemKVStore extends PuterStore { /** * Ensure each intermediate map layer exists for a set of nested paths. * Returns write units consumed. DDB can't set nested paths on missing - * parents in one expression, so we walk the layers and - * `SET ... if_not_exists(..., {})` each one. + * parents in one expression, so we walk the layers and `SET ... + * if_not_exists(..., {})` each one. */ private async createPaths( namespace: string, key: string, pathList: string[], ): Promise { + assertPaths(pathList); + const nestedMapValue = (() => { const valueRoot: Record = {}; let hasPaths = false; @@ -1039,12 +1115,12 @@ export class SystemKVStore extends PuterStore { let cursor: Record = valueRoot; for (let i = 0; i < chunks.length - 1; i++) { const chunk = chunks[i]; - const existing = cursor[chunk]; - if ( - !existing || - typeof existing !== 'object' || - Array.isArray(existing) - ) { + // Own properties only: an inherited hit here would mean + // walking (and then writing to) the prototype chain. + const existing = Object.hasOwn(cursor, chunk) + ? cursor[chunk] + : undefined; + if (!isPlainObject(existing)) { cursor[chunk] = {}; } cursor = cursor[chunk] as Record; diff --git a/src/gui/src/services/IPCService.js b/src/gui/src/services/IPCService.js index 4f8dc634a..5ff07e40f 100644 --- a/src/gui/src/services/IPCService.js +++ b/src/gui/src/services/IPCService.js @@ -44,29 +44,36 @@ export class IPCService extends Service { `; async _init () { - this.connections_ = {}; + // A Map, not an object: lookups are keyed by a uuid an app sends us + // (`messageToApp`), and on a plain object `constructor`/`__proto__` + // would resolve to an `Object.prototype` member — which we would then + // both treat as a live connection and write our cached instance onto. + // Matches ProcessService's uuid registries. + this.connections_ = new Map(); } add_connection ({ source, target }) { const uuid = window.uuidv4(); const r_uuid = window.uuidv4(); - const forward = this.connections_[uuid] = { + const forward = { source, target, uuid: uuid, reverse: r_uuid, }; - const backward = this.connections_[r_uuid] = { + const backward = { source: target, target: source, uuid: r_uuid, reverse: uuid, }; + this.connections_.set(uuid, forward); + this.connections_.set(r_uuid, backward); return { forward, backward }; } get_connection (uuid) { - const entry = this.connections_[uuid]; + const entry = this.connections_.get(uuid); if ( ! entry ) return; if ( entry.object ) return entry.object; return entry.object = new InternalConnection(entry, this.context); diff --git a/src/gui/src/services/IPCService.test.js b/src/gui/src/services/IPCService.test.js new file mode 100644 index 000000000..7ade70368 --- /dev/null +++ b/src/gui/src/services/IPCService.test.js @@ -0,0 +1,86 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest'; + +// `definitions.js` reaches for the class registry the GUI bundle installs in +// init_sync.js, so stub it before importing anything from that graph. +globalThis.def = globalThis.def ?? ((cls) => cls); +globalThis.use = globalThis.use ?? (() => ({})); + +let IPCService; +let uuidCounter = 0; + +beforeAll(async () => { + ({ IPCService } = await import('./IPCService.js')); +}); + +const makeService = async () => { + const svc = new IPCService(); + await svc.init({ services: { get: () => ({}) } }); + return svc; +}; + +describe('IPCService connections', () => { + beforeEach(() => { + uuidCounter = 0; + globalThis.window = globalThis.window ?? {}; + window.uuidv4 = () => `uuid-${++uuidCounter}`; + window.ipc_handlers = {}; + }); + + afterEach(() => { + delete Object.prototype.object; + }); + + it('round-trips a registered connection by uuid', async () => { + const svc = await makeService(); + const { forward, backward } = svc.add_connection({ + source: 'a', + target: 'b', + }); + expect(svc.get_connection(forward.uuid).target).toBe('b'); + expect(svc.get_connection(backward.uuid).target).toBe('a'); + }); + + it('caches the connection object per uuid', async () => { + const svc = await makeService(); + const { forward } = svc.add_connection({ source: 'a', target: 'b' }); + expect(svc.get_connection(forward.uuid)).toBe( + svc.get_connection(forward.uuid), + ); + }); + + // The uuid comes from an app's `messageToApp` payload, so a prototype + // member name must not look like a live connection — nor become one. + it.each(['__proto__', 'constructor', 'toString'])( + 'treats `%s` as an unknown connection', + async (uuid) => { + const svc = await makeService(); + svc.add_connection({ source: 'a', target: 'b' }); + expect(svc.get_connection(uuid)).toBeUndefined(); + expect({}.object).toBeUndefined(); + }, + ); + + it('returns undefined for an unknown uuid', async () => { + const svc = await makeService(); + expect(svc.get_connection('nope')).toBeUndefined(); + }); +}); diff --git a/src/puter-js/src/lib/xdrpc.js b/src/puter-js/src/lib/xdrpc.js index 4b967eafd..287ddbc40 100644 --- a/src/puter-js/src/lib/xdrpc.js +++ b/src/puter-js/src/lib/xdrpc.js @@ -6,6 +6,29 @@ // Since `Symbol` is not clonable, we use a UUID to identify RPCs. export const $SCOPE = '9a9c83a4-7897-43a0-93b9-53217b84fde6'; +/** + * Copy a key onto a rebuilt object as an own data property. + * + * Both sides of this bridge rebuild objects out of values the *other* + * document sent, so `result[key] = value` is not safe: a message carrying a + * literal `__proto__` key would hit `Object.prototype`'s setter and swap the + * prototype of the object we hand to our own caller, making it look like it + * has properties it was never sent. `defineProperty` always writes data. + * + * @param {Record} target + * @param {string} key + * @param {unknown} value + * @returns {void} + */ +const defineOwn = (target, key, value) => { + Object.defineProperty(target, key, { + value, + writable: true, + enumerable: true, + configurable: true, + }); +}; + /** * The CallbackManager is used to manage callbacks for RPCs. * It is used by the dehydrator and hydrator to store and retrieve @@ -59,8 +82,8 @@ export class Dehydrator { return value.map(this.dehydrate_value_.bind(this)); } else if ( typeof value === 'object' && value !== null ) { const result = {}; - for ( const key in value ) { - result[key] = this.dehydrate_value_(value[key]); + for ( const key of Object.keys(value) ) { + defineOwn(result, key, this.dehydrate_value_(value[key])); } return result; } else { @@ -94,8 +117,8 @@ export class Hydrator { return value.map(this.hydrate_value_.bind(this)); } else if ( typeof value === 'object' && value !== null ) { const result = {}; - for ( const key in value ) { - result[key] = this.hydrate_value_(value[key]); + for ( const key of Object.keys(value) ) { + defineOwn(result, key, this.hydrate_value_(value[key])); } return result; } diff --git a/src/puter-js/src/lib/xdrpc.test.js b/src/puter-js/src/lib/xdrpc.test.js new file mode 100644 index 000000000..7c9d9efb6 --- /dev/null +++ b/src/puter-js/src/lib/xdrpc.test.js @@ -0,0 +1,71 @@ +import { describe, expect, it } from 'vitest'; +import { CallbackManager, Dehydrator, Hydrator } from './xdrpc.js'; + +/** + * Build `{ [key]: value }` with `key` as a real own property — an object + * literal can't spell that for `__proto__`, but a message from the other + * document (or any JSON payload) carries it just fine. + */ +const withOwnKey = (key, value) => + JSON.parse(`{"${key}":${JSON.stringify(value)}}`); + +describe('xdrpc prototype safety', () => { + const prototypeKeys = ['__proto__', 'constructor', 'toString']; + + it.each(prototypeKeys)( + 'hydrate keeps a `%s` key as data on the rebuilt object', + (key) => { + const hydrator = new Hydrator({ target: { postMessage: () => {} } }); + const result = hydrator.hydrate( + withOwnKey(key, { escalated: true }), + ); + expect(Object.getPrototypeOf(result)).toBe(Object.prototype); + expect(result.escalated).toBeUndefined(); + expect(Object.hasOwn(result, key)).toBe(true); + }, + ); + + it.each(prototypeKeys)( + 'dehydrate keeps a `%s` key as data on the rebuilt object', + (key) => { + const dehydrator = new Dehydrator({ + callbackManager: new CallbackManager(), + }); + const result = dehydrator.dehydrate( + withOwnKey(key, { escalated: true }), + ); + expect(Object.getPrototypeOf(result)).toBe(Object.prototype); + expect(result.escalated).toBeUndefined(); + expect(Object.hasOwn(result, key)).toBe(true); + }, + ); + + it('leaves the shared prototype untouched', () => { + const hydrator = new Hydrator({ target: { postMessage: () => {} } }); + hydrator.hydrate(withOwnKey('__proto__', { escalated: true })); + expect({}.escalated).toBeUndefined(); + }); + + it('still round-trips ordinary values and callback stubs', () => { + const callbackManager = new CallbackManager(); + const dehydrated = new Dehydrator({ callbackManager }).dehydrate({ + name: 'x', + nested: { list: [1, 2], fn: () => 'called' }, + }); + expect(dehydrated).toMatchObject({ + name: 'x', + nested: { list: [1, 2] }, + }); + + const posted = []; + const hydrated = new Hydrator({ + target: { postMessage: (msg) => posted.push(msg) }, + }).hydrate(dehydrated); + expect(hydrated.name).toBe('x'); + expect(hydrated.nested.list).toEqual([1, 2]); + + hydrated.nested.fn('arg'); + expect(posted).toHaveLength(1); + expect(posted[0].args).toEqual(['arg']); + }); +}); diff --git a/src/puter-js/tests/api/suites/kv.suite.ts b/src/puter-js/tests/api/suites/kv.suite.ts index 5fa70cede..e023131a0 100644 --- a/src/puter-js/tests/api/suites/kv.suite.ts +++ b/src/puter-js/tests/api/suites/kv.suite.ts @@ -195,6 +195,38 @@ export default suite('kv', { t.assert.equal(await t.puter.kv.incr('kv-suite-incr-fresh', 3), 3); }, + 'incr rejects a path that walks the prototype chain': async (t) => { + await t.assert.rejects( + () => + t.puter.kv.incr('kv-suite-proto', { + 'constructor.prototype.polluted': 1, + }), + 'a prototype-walking path should be rejected', + ); + await t.assert.rejects( + () => + t.puter.kv.incr('kv-suite-proto', { + '__proto__.polluted.deep': 1, + }), + 'a __proto__ path should be rejected', + ); + t.assert.equal( + ({} as Record).polluted, + undefined, + 'nothing should have landed on Object.prototype', + ); + }, + + 'set rejects a value carrying a reserved object key': async (t) => { + // A JSON body can carry `__proto__` as a real own property; an object + // literal in source cannot. + const value = JSON.parse('{"__proto__":{"polluted":true}}'); + await t.assert.rejects( + () => t.puter.kv.set('kv-suite-proto-value', value), + 'a value with a __proto__ key should be rejected', + ); + }, + 'decr can drive a value negative': async (t) => { t.assert.equal(await t.puter.kv.decr('kv-suite-decr-neg', 5), -5); },