From 059129b6f73e2cc4b0565c798482c80442f9e6f7 Mon Sep 17 00:00:00 2001 From: Slava Yultyyev Date: Thu, 12 Nov 2026 21:30:03 -0800 Subject: [PATCH] fix(ui): reject cross-site and DNS-rebinding requests to the clip ui API The local API answered any request that reached 127.0.0.1:5573. A web page the user visits could send a text/plain or Content-Type-less POST (no CORS preflight), which the JSON-parsing handlers accepted, and a DNS-rebinding page could read and write as if it were same-origin. `/api/*` now rejects requests whose Host name isn't 127.0.0.1 or localhost, and runs Hono's built-in `csrf()` so cross-site writes are refused. The Vite dev proxy keeps working: it forwards the browser's own localhost Host and Origin. Co-Authored-By: Claude Opus 5.5 --- src/ui/server.ts | 27 ++++++ tests/ui-request-guard.test.ts | 148 +++++++++++++++++++++++++++++++++ 2 files changed, 175 insertions(+) create mode 100644 tests/ui-request-guard.test.ts diff --git a/src/ui/server.ts b/src/ui/server.ts index c55002b..9c14a7c 100644 --- a/src/ui/server.ts +++ b/src/ui/server.ts @@ -9,6 +9,7 @@ import { serve } from '@hono/node-server'; import { serveStatic } from '@hono/node-server/serve-static'; import getPort from 'get-port'; import { Hono } from 'hono'; +import { csrf } from 'hono/csrf'; import open from 'open'; import { ZodError, z } from 'zod'; import { buildThumbnailArgs } from '../ffmpeg/args/preview.js'; @@ -86,6 +87,19 @@ function findStaticDir(): string | null { return null; } +const LOOPBACK_HOSTNAMES = new Set(['127.0.0.1', 'localhost']); + +/** Any port is fine: the Vite dev proxy forwards the browser's own Host + * (`localhost:5173`). Only the name matters for DNS rebinding. */ +function isLoopbackHost(host: string | undefined): boolean { + if (!host) return false; + try { + return LOOPBACK_HOSTNAMES.has(new URL(`http://${host}`).hostname); + } catch { + return false; + } +} + export interface UiServerOptions { /** Preferred port. Auto-picks a free port if this one is taken. */ port?: number; @@ -129,6 +143,19 @@ export async function startUiServer(options: UiServerOptions = {}): Promise { + if (!isLoopbackHost(c.req.header('host'))) { + return c.json({ error: 'Forbidden: clip ui only answers to 127.0.0.1 or localhost.' }, 403); + } + await next(); + }); + app.use('/api/*', csrf()); + app.get('/api/workspace', async (c) => { return c.json({ path: getWorkspace() }); }); diff --git a/tests/ui-request-guard.test.ts b/tests/ui-request-guard.test.ts new file mode 100644 index 0000000..216ff22 --- /dev/null +++ b/tests/ui-request-guard.test.ts @@ -0,0 +1,148 @@ +import { mkdtemp, rm } from 'node:fs/promises'; +import { request } from 'node:http'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { startUiServer, type UiServer } from '../src/ui/server.js'; + +interface Reply { + status: number; + body: string; +} + +// node:http rather than fetch: the tests need to set Host and Origin, which +// fetch treats as forbidden request headers. +function send( + port: number, + method: string, + path: string, + headers: Record, + body?: string, +): Promise { + return new Promise((resolveReply, reject) => { + const req = request({ host: '127.0.0.1', port, method, path, headers }, (res) => { + let data = ''; + res.setEncoding('utf8'); + res.on('data', (chunk: string) => { + data += chunk; + }); + res.on('end', () => resolveReply({ status: res.statusCode ?? 0, body: data })); + }); + req.on('error', reject); + req.end(body); + }); +} + +let server: UiServer; +let workspace: string; +let savedWorkspace: string | undefined; + +beforeAll(async () => { + workspace = await mkdtemp(join(tmpdir(), 'mmc-ui-guard-test-')); + savedWorkspace = process.env.MAKEMYCLIP_WORKSPACE; + process.env.MAKEMYCLIP_WORKSPACE = workspace; + server = await startUiServer({ openBrowser: false }); +}); + +afterAll(async () => { + await server.stop(); + if (savedWorkspace === undefined) { + delete process.env.MAKEMYCLIP_WORKSPACE; + } else { + process.env.MAKEMYCLIP_WORKSPACE = savedWorkspace; + } + await rm(workspace, { recursive: true, force: true }); +}); + +describe('clip ui request guard', () => { + it('serves a same-origin write from the UI', async () => { + const host = `127.0.0.1:${server.port}`; + const reply = await send( + server.port, + 'POST', + '/api/session/snapshot', + { host, origin: `http://${host}`, 'content-type': 'text/plain;charset=UTF-8' }, + '{}', + ); + expect(reply.status).toBe(200); + }); + + it('serves a write through the Vite dev proxy, which keeps the browser Host', async () => { + const reply = await send( + server.port, + 'POST', + '/api/session/snapshot', + { + host: 'localhost:5173', + origin: 'http://localhost:5173', + 'content-type': 'application/json', + }, + '{}', + ); + expect(reply.status).toBe(200); + }); + + it('serves a JSON write from a non-browser client that sends no Origin', async () => { + const reply = await send( + server.port, + 'POST', + '/api/session/snapshot', + { host: `127.0.0.1:${server.port}`, 'content-type': 'application/json' }, + '{}', + ); + expect(reply.status).toBe(200); + }); + + it('rejects a cross-site text/plain write (no CORS preflight)', async () => { + const reply = await send( + server.port, + 'POST', + '/api/session/snapshot', + { + host: `127.0.0.1:${server.port}`, + origin: 'https://evil.example', + 'content-type': 'text/plain;charset=UTF-8', + }, + '{"label":"from-evil"}', + ); + expect(reply.status).toBe(403); + }); + + it('rejects a cross-site write that sends no Content-Type', async () => { + const reply = await send( + server.port, + 'POST', + '/api/session/snapshot', + { host: `127.0.0.1:${server.port}`, origin: 'https://evil.example' }, + '{"label":"from-evil"}', + ); + expect(reply.status).toBe(403); + }); + + it('rejects a read whose Host is not a loopback name (DNS rebinding)', async () => { + const reply = await send(server.port, 'GET', '/api/workspace', { + host: `evil.example:${server.port}`, + }); + expect(reply.status).toBe(403); + expect(reply.body).not.toContain(workspace); + }); + + it('rejects a same-origin write from a rebound Host', async () => { + const host = `evil.example:${server.port}`; + const reply = await send( + server.port, + 'POST', + '/api/session/snapshot', + { host, origin: `http://${host}`, 'content-type': 'text/plain;charset=UTF-8' }, + '{}', + ); + expect(reply.status).toBe(403); + }); + + it('serves a read addressed to localhost', async () => { + const reply = await send(server.port, 'GET', '/api/workspace', { + host: `localhost:${server.port}`, + }); + expect(reply.status).toBe(200); + }); +});