diff --git a/CHANGELOG.md b/CHANGELOG.md index 5aca8e7a5b..77c0ff6af0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Security/Media: harden local media ingestion against TOCTOU/symlink swap attacks by pinning reads to a single file descriptor with symlink rejection and inode/device verification in `saveMediaSource`. Thanks @dorjoos for reporting. - Agents/Streaming: keep assistant partial streaming active during reasoning streams, handle native `thinking_*` stream events consistently, dedupe mixed reasoning-end signals, and clear stale mutating tool errors after same-target retry success. (#20635) Thanks @obviyus. - Gateway/Auth: default unresolved gateway auth to token mode with startup auto-generation/persistence of `gateway.auth.token`, while allowing explicit `gateway.auth.mode: "none"` for intentional open loopback setups. (#20686) thanks @gumadeiras. - Browser/Relay: require gateway-token auth on both `/extension` and `/cdp`, and align Chrome extension setup to use a single `gateway.auth.token` input for relay authentication. Thanks @tdjackey for reporting. diff --git a/src/media/store.test.ts b/src/media/store.test.ts index dbdc226bac..1486c75ce8 100644 --- a/src/media/store.test.ts +++ b/src/media/store.test.ts @@ -1,7 +1,7 @@ +import JSZip from "jszip"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import JSZip from "jszip"; import sharp from "sharp"; import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; import { isPathWithinBase } from "../../test/helpers/paths.js"; @@ -102,6 +102,17 @@ describe("media store", () => { }); }); + it.runIf(process.platform !== "win32")("rejects symlink sources", async () => { + await withTempStore(async (store, home) => { + const target = path.join(home, "sensitive.txt"); + const source = path.join(home, "source.txt"); + await fs.writeFile(target, "sensitive"); + await fs.symlink(target, source); + + await expect(store.saveMediaSource(source)).rejects.toThrow("symlink"); + }); + }); + it("cleans old media files in first-level subdirectories", async () => { await withTempStore(async (store) => { const saved = await store.saveMediaBuffer(Buffer.from("nested"), "text/plain", "inbound"); diff --git a/src/media/store.ts b/src/media/store.ts index c5882ae10f..b4c4ead54d 100644 --- a/src/media/store.ts +++ b/src/media/store.ts @@ -1,4 +1,5 @@ import crypto from "node:crypto"; +import { constants as fsConstants } from "node:fs"; import { createWriteStream } from "node:fs"; import fs from "node:fs/promises"; import { request as httpRequest } from "node:http"; @@ -20,6 +21,12 @@ const defaultHttpRequestImpl: RequestImpl = httpRequest; const defaultHttpsRequestImpl: RequestImpl = httpsRequest; const defaultResolvePinnedHostnameImpl: ResolvePinnedHostnameImpl = resolvePinnedHostname; +const isNodeError = (err: unknown): err is NodeJS.ErrnoException => + Boolean(err && typeof err === "object" && "code" in (err as Record)); + +const isSymlinkOpenError = (err: unknown) => + isNodeError(err) && (err.code === "ELOOP" || err.code === "EINVAL" || err.code === "ENOTSUP"); + let httpRequestImpl: RequestImpl = defaultHttpRequestImpl; let httpsRequestImpl: RequestImpl = defaultHttpsRequestImpl; let resolvePinnedHostnameImpl: ResolvePinnedHostnameImpl = defaultResolvePinnedHostnameImpl; @@ -232,20 +239,41 @@ export async function saveMediaSource( return { id, path: finalDest, size, contentType: mime }; } // local path - const stat = await fs.stat(source); - if (!stat.isFile()) { - throw new Error("Media path is not a file"); + const supportsNoFollow = process.platform !== "win32" && "O_NOFOLLOW" in fsConstants; + const flags = fsConstants.O_RDONLY | (supportsNoFollow ? fsConstants.O_NOFOLLOW : 0); + let handle: Awaited>; + try { + handle = await fs.open(source, flags); + } catch (err) { + if (isSymlinkOpenError(err)) { + throw new Error("Media path must not be a symlink", { cause: err }); + } + throw err; } - if (stat.size > MAX_BYTES) { - throw new Error("Media exceeds 5MB limit"); + try { + const [stat, lstat] = await Promise.all([handle.stat(), fs.lstat(source)]); + if (lstat.isSymbolicLink()) { + throw new Error("Media path must not be a symlink"); + } + if (!stat.isFile()) { + throw new Error("Media path is not a file"); + } + if (stat.ino !== lstat.ino || stat.dev !== lstat.dev) { + throw new Error("Media path changed during read"); + } + if (stat.size > MAX_BYTES) { + throw new Error("Media exceeds 5MB limit"); + } + const buffer = await handle.readFile(); + const mime = await detectMime({ buffer, filePath: source }); + const ext = extensionForMime(mime) ?? path.extname(source); + const id = ext ? `${baseId}${ext}` : baseId; + const dest = path.join(dir, id); + await fs.writeFile(dest, buffer, { mode: 0o600 }); + return { id, path: dest, size: stat.size, contentType: mime }; + } finally { + await handle.close().catch(() => {}); } - const buffer = await fs.readFile(source); - const mime = await detectMime({ buffer, filePath: source }); - const ext = extensionForMime(mime) ?? path.extname(source); - const id = ext ? `${baseId}${ext}` : baseId; - const dest = path.join(dir, id); - await fs.writeFile(dest, buffer, { mode: 0o600 }); - return { id, path: dest, size: stat.size, contentType: mime }; } export async function saveMediaBuffer(