From 4e9b03bf5c2a2eda8499b866f0c186617a772dfb Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Mon, 21 Sep 2026 09:19:37 -0700 Subject: [PATCH] fix: harden clickjacking (#3912) --- src/puter-js/src/lib/xdrpc.js | 55 ++++++++++++++------ src/puter-js/src/lib/xdrpc.test.js | 57 +++++++++++++++++++++ src/puter-js/src/modules/UI.js | 12 ++++- src/puter-js/src/modules/Util.js | 18 ++++--- src/puter-js/tests/api/suites/util.suite.ts | 18 ++++--- 5 files changed, 132 insertions(+), 28 deletions(-) diff --git a/src/puter-js/src/lib/xdrpc.js b/src/puter-js/src/lib/xdrpc.js index 287ddbc40..75a904815 100644 --- a/src/puter-js/src/lib/xdrpc.js +++ b/src/puter-js/src/lib/xdrpc.js @@ -29,34 +29,58 @@ const defineOwn = (target, key, value) => { }); }; +/** + * A callback id travels to the other document, so it must not be guessable: + * any window able to reach ours can post `$SCOPE` messages, and `$SCOPE` + * itself is a public constant. + * + * @returns {string} + */ +const randomCallbackId = () => { + const bytes = crypto.getRandomValues(new Uint8Array(16)); + return Array.from(bytes, b => b.toString(16).padStart(2, '0')).join(''); +}; + /** * The CallbackManager is used to manage callbacks for RPCs. * It is used by the dehydrator and hydrator to store and retrieve * the functions that are being called remotely. */ export class CallbackManager { - #messageId = 1; - constructor () { this.callbacks = new Map(); } - register_callback (callback) { - const id = this.#messageId++; - this.callbacks.set(id, callback); + /** + * Registers `callback` and binds it to `source`, the only window whose + * messages may invoke it later. A callback registered without a source + * can never be invoked from outside this document. + * + * @param {Function} callback + * @param {Window} [source] + * @returns {string} + */ + register_callback (callback, source) { + const id = randomCallbackId(); + this.callbacks.set(id, { callback, source }); return id; } - attach_to_source (source) { - source.addEventListener('message', event => { + /** + * @param {Window} target + * @returns {void} + */ + attach_to_source (target) { + target.addEventListener('message', event => { const { data } = event; - if ( data && typeof data === 'object' && data.$SCOPE === $SCOPE ) { - const { id, args } = data; - const callback = this.callbacks.get(id); - if ( callback ) { - callback(...args); - } + if ( ! data || typeof data !== 'object' || data.$SCOPE !== $SCOPE ) { + return; } + const entry = this.callbacks.get(data.id); + // Only the window the callback was dehydrated for may invoke it, + // otherwise a sibling frame could drive another app's callbacks. + if ( ! entry || event.source !== entry.source ) return; + entry.callback(...(Array.isArray(data.args) ? data.args : [])); }); } } @@ -68,15 +92,16 @@ export class CallbackManager { * so that they can be called when the RPC is invoked. */ export class Dehydrator { - constructor ({ callbackManager }) { + constructor ({ callbackManager, source }) { this.callbackManager = callbackManager; + this.source = source; } dehydrate (value) { return this.dehydrate_value_(value); } dehydrate_value_ (value) { if ( typeof value === 'function' ) { - const id = this.callbackManager.register_callback(value); + const id = this.callbackManager.register_callback(value, this.source); return { $SCOPE, id }; } else if ( Array.isArray(value) ) { return value.map(this.dehydrate_value_.bind(this)); diff --git a/src/puter-js/src/lib/xdrpc.test.js b/src/puter-js/src/lib/xdrpc.test.js index 7c9d9efb6..ef51d3206 100644 --- a/src/puter-js/src/lib/xdrpc.test.js +++ b/src/puter-js/src/lib/xdrpc.test.js @@ -69,3 +69,60 @@ describe('xdrpc prototype safety', () => { expect(posted[0].args).toEqual(['arg']); }); }); + +describe('xdrpc callback source binding', () => { + const $SCOPE = '9a9c83a4-7897-43a0-93b9-53217b84fde6'; + + /** A stand-in for `globalThis` that lets a test deliver a message event. */ + const fakeWindow = () => { + const handlers = []; + return { + addEventListener: (type, handler) => + type === 'message' && handlers.push(handler), + deliver: event => handlers.forEach(handler => handler(event)), + }; + }; + + const register = ({ source }) => { + const manager = new CallbackManager(); + const calls = []; + const id = manager.register_callback((...args) => calls.push(args), source); + const listener = fakeWindow(); + manager.attach_to_source(listener); + return { calls, id, listener }; + }; + + it('invokes a callback for a message from its registered source', () => { + const gui = {}; + const { calls, id, listener } = register({ source: gui }); + listener.deliver({ source: gui, data: { $SCOPE, id, args: ['ok'] } }); + expect(calls).toEqual([['ok']]); + }); + + it('ignores the same message from another window', () => { + const gui = {}; + const { calls, id, listener } = register({ source: gui }); + listener.deliver({ + source: { sibling: true }, + data: { $SCOPE, id, args: ['forged'] }, + }); + expect(calls).toEqual([]); + }); + + it('ignores a callback registered without a source', () => { + const { calls, id, listener } = register({ source: undefined }); + listener.deliver({ source: {}, data: { $SCOPE, id, args: [] } }); + expect(calls).toEqual([]); + }); + + it('hands out ids that cannot be guessed from an earlier one', () => { + const manager = new CallbackManager(); + const ids = Array.from({ length: 5 }, () => + manager.register_callback(() => {}, {}), + ); + expect(new Set(ids).size).toBe(ids.length); + for ( const id of ids ) { + expect(id).toMatch(/^[0-9a-f]{32}$/); + } + }); +}); diff --git a/src/puter-js/src/modules/UI.js b/src/puter-js/src/modules/UI.js index ed7225fb1..040bd10a3 100644 --- a/src/puter-js/src/modules/UI.js +++ b/src/puter-js/src/modules/UI.js @@ -350,6 +350,11 @@ export class AppConnection extends EventListener { // TODO: Set this.#puterOrigin to the puter origin (globalThis.document) && window.addEventListener('message', event => { + // Relayed by the host environment; a window that guessed an + // appInstanceID must not be able to forge one directly. + if ( event.source !== this.messageTarget ) return; + if ( ! event.data ) return; + if ( event.data.msg === 'messageToApp' ) { if ( event.data.appInstanceID !== this.targetAppInstanceID ) { // Message is from a different AppConnection; ignore it. @@ -577,7 +582,7 @@ export class UIModule extends EventListener { done_setting_resolve(); }); }); - const callback_id = this.util.rpc.registerCallback(resolve); + const callback_id = this.util.rpc.registerCallback(resolve, this.messageTarget); this.messageTarget?.postMessage({ $: 'puter-ipc', v: 2, @@ -648,6 +653,11 @@ export class UIModule extends EventListener { // Bind the message event listener to the window let lastDraggedOverElement = null; (globalThis.document) && window.addEventListener('message', async (e) => { + // Only the host environment drives these. Pinning the source + // rather than the origin keeps locally-hosted and self-hosted + // deployments working, and still rejects a sibling app iframe or + // a third-party page that framed us. + if ( e.source !== this.messageTarget ) return; if ( ! e.data ) return; // `error` if ( e.data.error ) { diff --git a/src/puter-js/src/modules/Util.js b/src/puter-js/src/modules/Util.js index 1ddd1acc5..f2bd9ff4b 100644 --- a/src/puter-js/src/modules/Util.js +++ b/src/puter-js/src/modules/Util.js @@ -27,12 +27,17 @@ export class UtilRPC { /** * A dehydrator that replaces functions in a value with callback ids this - * side can later resolve, so the value survives `postMessage`. + * side can later resolve, so the value survives `postMessage`. Only + * `target` may invoke the callbacks it registers. * + * @param {{ target?: Window }} [config] * @returns {{ dehydrate: (value: unknown) => unknown }} */ - getDehydrator () { - return new Dehydrator({ callbackManager: this.callbackManager }); + getDehydrator ({ target } = {}) { + return new Dehydrator({ + callbackManager: this.callbackManager, + source: target, + }); } /** @@ -47,13 +52,14 @@ export class UtilRPC { } /** - * Registers a function under a callback id the other side can invoke. + * Registers a function under a callback id `source` can invoke. * * @param {(value: unknown) => void} resolve + * @param {Window} [source] * @returns {string} */ - registerCallback (resolve) { - return this.callbackManager.register_callback(resolve); + registerCallback (resolve, source) { + return this.callbackManager.register_callback(resolve, source); } /** diff --git a/src/puter-js/tests/api/suites/util.suite.ts b/src/puter-js/tests/api/suites/util.suite.ts index 0dad9b809..6891df6e0 100644 --- a/src/puter-js/tests/api/suites/util.suite.ts +++ b/src/puter-js/tests/api/suites/util.suite.ts @@ -21,12 +21,14 @@ type PuterInternals = { }; util: { rpc: { - getDehydrator: () => { dehydrate: (value: unknown) => never }; + getDehydrator: (config?: { target: unknown }) => { + dehydrate: (value: unknown) => never; + }; getHydrator: (config: { target: unknown }) => { hydrate: (value: unknown) => never; }; - registerCallback: (fn: () => void) => number; - send: (target: unknown, id: number, ...args: unknown[]) => void; + registerCallback: (fn: () => void, source?: unknown) => string; + send: (target: unknown, id: string, ...args: unknown[]) => void; }; }; }; @@ -285,14 +287,18 @@ export default suite('util', { list: [() => {}, 'literal'], }) as unknown as { plain: number; - callback: { $SCOPE: string; id: number }; - list: [{ $SCOPE: string; id: number }, string]; + callback: { $SCOPE: string; id: string }; + list: [{ $SCOPE: string; id: string }, string]; }; t.assert.equal(dehydrated.plain, 1, 'plain values survive untouched'); t.assert.equal(dehydrated.list[1], 'literal'); t.assert.equal(typeof dehydrated.callback.$SCOPE, 'string'); - t.assert.equal(typeof dehydrated.callback.id, 'number'); + t.assert.equal(typeof dehydrated.callback.id, 'string'); + t.assert.ok( + /^[0-9a-f]{32}$/.test(dehydrated.callback.id), + 'callback ids are random, not a guessable counter', + ); t.assert.ok( dehydrated.callback.id !== dehydrated.list[0].id, 'each function gets its own callback id',