fix: harden clickjacking (#3912)

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));

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}$/);
+        }
+    });
+});

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 ) {

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);
     }
 
     /**

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',