From 7c500ff6236fa087ec1ec88696ca9f6881e90dc5 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 21 Feb 2026 23:10:47 +0100 Subject: [PATCH] fix(security): harden control-ui static path resolution --- src/gateway/control-ui.http.test.ts | 83 +++++++++++++++++++++++++++++ src/gateway/control-ui.ts | 80 ++++++++++++++++++++++++--- 2 files changed, 155 insertions(+), 8 deletions(-) diff --git a/src/gateway/control-ui.http.test.ts b/src/gateway/control-ui.http.test.ts index 20b0503e9..a2f2fe52f 100644 --- a/src/gateway/control-ui.http.test.ts +++ b/src/gateway/control-ui.http.test.ts @@ -125,4 +125,87 @@ describe("handleControlUiHttpRequest", () => { }, }); }); + + it("rejects symlinked assets that resolve outside control-ui root", async () => { + await withControlUiRoot({ + fn: async (tmp) => { + const assetsDir = path.join(tmp, "assets"); + const outsideDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-ui-outside-")); + try { + const outsideFile = path.join(outsideDir, "secret.txt"); + await fs.mkdir(assetsDir, { recursive: true }); + await fs.writeFile(outsideFile, "outside-secret\n"); + await fs.symlink(outsideFile, path.join(assetsDir, "leak.txt")); + + const { res, end } = makeMockHttpResponse(); + const handled = handleControlUiHttpRequest( + { url: "/assets/leak.txt", method: "GET" } as IncomingMessage, + res, + { + root: { kind: "resolved", path: tmp }, + }, + ); + + expect(handled).toBe(true); + expect(res.statusCode).toBe(404); + expect(end).toHaveBeenCalledWith("Not Found"); + } finally { + await fs.rm(outsideDir, { recursive: true, force: true }); + } + }, + }); + }); + + it("allows symlinked assets that resolve inside control-ui root", async () => { + await withControlUiRoot({ + fn: async (tmp) => { + const assetsDir = path.join(tmp, "assets"); + await fs.mkdir(assetsDir, { recursive: true }); + await fs.writeFile(path.join(assetsDir, "actual.txt"), "inside-ok\n"); + await fs.symlink(path.join(assetsDir, "actual.txt"), path.join(assetsDir, "linked.txt")); + + const { res, end } = makeMockHttpResponse(); + const handled = handleControlUiHttpRequest( + { url: "/assets/linked.txt", method: "GET" } as IncomingMessage, + res, + { + root: { kind: "resolved", path: tmp }, + }, + ); + + expect(handled).toBe(true); + expect(res.statusCode).toBe(200); + expect(String(end.mock.calls[0]?.[0] ?? "")).toBe("inside-ok\n"); + }, + }); + }); + + it("rejects symlinked SPA fallback index.html outside control-ui root", async () => { + await withControlUiRoot({ + fn: async (tmp) => { + const outsideDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-ui-index-outside-")); + try { + const outsideIndex = path.join(outsideDir, "index.html"); + await fs.writeFile(outsideIndex, "outside\n"); + await fs.rm(path.join(tmp, "index.html")); + await fs.symlink(outsideIndex, path.join(tmp, "index.html")); + + const { res, end } = makeMockHttpResponse(); + const handled = handleControlUiHttpRequest( + { url: "/app/route", method: "GET" } as IncomingMessage, + res, + { + root: { kind: "resolved", path: tmp }, + }, + ); + + expect(handled).toBe(true); + expect(res.statusCode).toBe(404); + expect(end).toHaveBeenCalledWith("Not Found"); + } finally { + await fs.rm(outsideDir, { recursive: true, force: true }); + } + }, + }); + }); }); diff --git a/src/gateway/control-ui.ts b/src/gateway/control-ui.ts index 4b05f1e34..08bc2500b 100644 --- a/src/gateway/control-ui.ts +++ b/src/gateway/control-ui.ts @@ -188,10 +188,72 @@ function serveFile(res: ServerResponse, filePath: string) { res.end(fs.readFileSync(filePath)); } -function serveIndexHtml(res: ServerResponse, indexPath: string) { +function serveResolvedFile(res: ServerResponse, filePath: string, body: Buffer) { + const ext = path.extname(filePath).toLowerCase(); + res.setHeader("Content-Type", contentTypeForExt(ext)); + res.setHeader("Cache-Control", "no-cache"); + res.end(body); +} + +function serveResolvedIndexHtml(res: ServerResponse, body: string) { res.setHeader("Content-Type", "text/html; charset=utf-8"); res.setHeader("Cache-Control", "no-cache"); - res.end(fs.readFileSync(indexPath, "utf8")); + res.end(body); +} + +function isContainedPath(baseDir: string, targetPath: string): boolean { + const relative = path.relative(baseDir, targetPath); + return relative !== ".." && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative); +} + +function isExpectedSafePathError(error: unknown): boolean { + const code = + typeof error === "object" && error !== null && "code" in error ? String(error.code) : ""; + return code === "ENOENT" || code === "ENOTDIR" || code === "ELOOP"; +} + +function areSameFileIdentity(preOpen: fs.Stats, opened: fs.Stats): boolean { + return preOpen.dev === opened.dev && preOpen.ino === opened.ino; +} + +function resolveSafeControlUiFile( + root: string, + filePath: string, +): { path: string; body: Buffer } | null { + let fd: number | null = null; + try { + const rootReal = fs.realpathSync(root); + const fileReal = fs.realpathSync(filePath); + if (!isContainedPath(rootReal, fileReal)) { + return null; + } + + const preOpenStat = fs.lstatSync(fileReal); + if (!preOpenStat.isFile()) { + return null; + } + + const openFlags = + fs.constants.O_RDONLY | + (typeof fs.constants.O_NOFOLLOW === "number" ? fs.constants.O_NOFOLLOW : 0); + fd = fs.openSync(fileReal, openFlags); + const openedStat = fs.fstatSync(fd); + // Compare inode identity so swaps between validation and open are rejected. + if (!openedStat.isFile() || !areSameFileIdentity(preOpenStat, openedStat)) { + return null; + } + + return { path: fileReal, body: fs.readFileSync(fd) }; + } catch (error) { + if (isExpectedSafePathError(error)) { + return null; + } + throw error; + } finally { + if (fd !== null) { + fs.closeSync(fd); + } + } } function isSafeRelativePath(relPath: string) { @@ -340,12 +402,13 @@ export function handleControlUiHttpRequest( return true; } - if (fs.existsSync(filePath) && fs.statSync(filePath).isFile()) { - if (path.basename(filePath) === "index.html") { - serveIndexHtml(res, filePath); + const safeFile = resolveSafeControlUiFile(root, filePath); + if (safeFile) { + if (path.basename(safeFile.path) === "index.html") { + serveResolvedIndexHtml(res, safeFile.body.toString("utf8")); return true; } - serveFile(res, filePath); + serveResolvedFile(res, safeFile.path, safeFile.body); return true; } @@ -361,8 +424,9 @@ export function handleControlUiHttpRequest( // SPA fallback (client-side router): serve index.html for unknown paths. const indexPath = path.join(root, "index.html"); - if (fs.existsSync(indexPath)) { - serveIndexHtml(res, indexPath); + const safeIndex = resolveSafeControlUiFile(root, indexPath); + if (safeIndex) { + serveResolvedIndexHtml(res, safeIndex.body.toString("utf8")); return true; }