From 8e6d1e636821141349da4f9d96fcc3c924449c8e Mon Sep 17 00:00:00 2001 From: Mariano <132747814+mbelinky@users.noreply.github.com> Date: Thu, 19 Feb 2026 09:37:33 +0000 Subject: [PATCH] LINE/Security: harden inbound media temp-file naming (#20792) Merged via /review-pr -> /prepare-pr -> /merge-pr. Prepared head SHA: f6f3eecdb3ce21ccdccd8b2c10a74ebd47ad809e Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com> Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com> Reviewed-by: @mbelinky --- CHANGELOG.md | 1 + src/line/download.test.ts | 69 +++++++++++++++++++++++++++++++++++++++ src/line/download.ts | 11 ++++--- 3 files changed, 77 insertions(+), 4 deletions(-) create mode 100644 src/line/download.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index fa7d47dcf..e1da32c2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Docs: https://docs.openclaw.ai ### Fixes - Security/Feishu: prevent path traversal in Feishu inbound media temp-file writes by replacing key-derived temp filenames with UUID-based names. Thanks @allsmog for reporting. +- LINE/Security: harden inbound media temp-file naming by using UUID-based temp paths for downloaded media instead of external message IDs. (#20792) Thanks @mbelinky. - 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. - Security/Lobster (Windows): for the next npm release, remove shell-based fallback when launching Lobster wrappers (`.cmd`/`.bat`) and switch to explicit argv execution with wrapper entrypoint resolution, preventing command injection while preserving Windows wrapper compatibility. Thanks @tdjackey 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. diff --git a/src/line/download.test.ts b/src/line/download.test.ts new file mode 100644 index 000000000..2e64473b7 --- /dev/null +++ b/src/line/download.test.ts @@ -0,0 +1,69 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const getMessageContentMock = vi.hoisted(() => vi.fn()); + +vi.mock("@line/bot-sdk", () => ({ + messagingApi: { + MessagingApiBlobClient: class { + getMessageContent(messageId: string) { + return getMessageContentMock(messageId); + } + }, + }, +})); + +vi.mock("../globals.js", () => ({ + logVerbose: () => {}, +})); + +import { downloadLineMedia } from "./download.js"; + +async function* chunks(parts: Buffer[]): AsyncGenerator { + for (const part of parts) { + yield part; + } +} + +describe("downloadLineMedia", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("does not derive temp file path from external messageId", async () => { + const messageId = "a/../../../../etc/passwd"; + const jpeg = Buffer.from([0xff, 0xd8, 0xff, 0x00]); + getMessageContentMock.mockResolvedValueOnce(chunks([jpeg])); + + const writeSpy = vi.spyOn(fs.promises, "writeFile").mockResolvedValueOnce(undefined); + + const result = await downloadLineMedia(messageId, "token"); + const writtenPath = writeSpy.mock.calls[0]?.[0]; + + expect(result.size).toBe(jpeg.length); + expect(result.contentType).toBe("image/jpeg"); + expect(typeof writtenPath).toBe("string"); + if (typeof writtenPath !== "string") { + throw new Error("expected string temp file path"); + } + expect(result.path).toBe(writtenPath); + expect(writtenPath).toContain("line-media-"); + expect(writtenPath).toMatch(/\.jpg$/); + expect(writtenPath).not.toContain(messageId); + expect(writtenPath).not.toContain(".."); + + const tmpRoot = path.resolve(os.tmpdir()); + const rel = path.relative(tmpRoot, path.resolve(writtenPath)); + expect(rel === ".." || rel.startsWith(`..${path.sep}`)).toBe(false); + }); + + it("rejects oversized media before writing to disk", async () => { + getMessageContentMock.mockResolvedValueOnce(chunks([Buffer.alloc(4), Buffer.alloc(4)])); + const writeSpy = vi.spyOn(fs.promises, "writeFile").mockResolvedValue(undefined); + + await expect(downloadLineMedia("mid", "token", 7)).rejects.toThrow(/Media exceeds/i); + expect(writeSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/src/line/download.ts b/src/line/download.ts index 064662d71..f61e4f936 100644 --- a/src/line/download.ts +++ b/src/line/download.ts @@ -1,3 +1,4 @@ +import crypto from "node:crypto"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; @@ -10,6 +11,10 @@ interface DownloadResult { size: number; } +function buildLineTempMediaPath(extension: string): string { + return path.join(os.tmpdir(), `line-media-${Date.now()}-${crypto.randomUUID()}${extension}`); +} + export async function downloadLineMedia( messageId: string, channelAccessToken: string, @@ -39,10 +44,8 @@ export async function downloadLineMedia( const contentType = detectContentType(buffer); const ext = getExtensionForContentType(contentType); - // Write to temp file - const tempDir = os.tmpdir(); - const fileName = `line-media-${messageId}-${Date.now()}${ext}`; - const filePath = path.join(tempDir, fileName); + // Use random temp names; never derive paths from external message identifiers. + const filePath = buildLineTempMediaPath(ext); await fs.promises.writeFile(filePath, buffer);