mirror of https://github.com/openclaw/openclaw.git
fix(imessage): sanitize SCP remote path to prevent shell metacharacter injection
References GHSA-g2f6-pwvx-r275.
This commit is contained in:
parent
ff6636ed5b
commit
a54bf71b4c
|
|
@ -14,6 +14,7 @@ Docs: https://docs.openclaw.ai
|
|||
|
||||
### Fixes
|
||||
|
||||
- iMessage/remote attachments: reject unsafe remote attachment paths before spawning SCP, so sender-controlled filenames can no longer inject shell metacharacters into remote media staging. Thanks @lintsinghua.
|
||||
- Telegram/webhook auth: validate the Telegram webhook secret before reading or parsing request bodies, so unauthenticated requests are rejected immediately instead of consuming up to 1 MB first. Thanks @space08.
|
||||
- Build/plugin-sdk bundling: bundle plugin-sdk subpath entries in one shared build pass so published packages stop duplicating shared chunks and avoid the recent plugin-sdk memory blow-up. (#45426) Thanks @TarasShyn.
|
||||
- Browser/existing-session: accept text-only `list_pages` and `new_page` responses from Chrome DevTools MCP so live-session tab discovery and new-tab open flows keep working when the server omits structured page metadata.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,75 @@
|
|||
import fs from "node:fs/promises";
|
||||
import { basename, join } from "node:path";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
createSandboxMediaContexts,
|
||||
createSandboxMediaStageConfig,
|
||||
withSandboxMediaTempHome,
|
||||
} from "./stage-sandbox-media.test-harness.js";
|
||||
|
||||
const sandboxMocks = vi.hoisted(() => ({
|
||||
ensureSandboxWorkspaceForSession: vi.fn(),
|
||||
}));
|
||||
const childProcessMocks = vi.hoisted(() => ({
|
||||
spawn: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../agents/sandbox.js", () => sandboxMocks);
|
||||
vi.mock("node:child_process", () => childProcessMocks);
|
||||
|
||||
import { stageSandboxMedia } from "./reply/stage-sandbox-media.js";
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
childProcessMocks.spawn.mockClear();
|
||||
});
|
||||
|
||||
function createRemoteStageParams(home: string): {
|
||||
cfg: ReturnType<typeof createSandboxMediaStageConfig>;
|
||||
workspaceDir: string;
|
||||
sessionKey: string;
|
||||
remoteCacheDir: string;
|
||||
} {
|
||||
const sessionKey = "agent:main:main";
|
||||
vi.mocked(sandboxMocks.ensureSandboxWorkspaceForSession).mockResolvedValue(null);
|
||||
return {
|
||||
cfg: createSandboxMediaStageConfig(home),
|
||||
workspaceDir: join(home, "openclaw"),
|
||||
sessionKey,
|
||||
remoteCacheDir: join(home, ".openclaw", "media", "remote-cache", sessionKey),
|
||||
};
|
||||
}
|
||||
|
||||
function createRemoteContexts(remotePath: string) {
|
||||
const { ctx, sessionCtx } = createSandboxMediaContexts(remotePath);
|
||||
ctx.Provider = "imessage";
|
||||
ctx.MediaRemoteHost = "user@gateway-host";
|
||||
sessionCtx.Provider = "imessage";
|
||||
sessionCtx.MediaRemoteHost = "user@gateway-host";
|
||||
return { ctx, sessionCtx };
|
||||
}
|
||||
|
||||
describe("stageSandboxMedia scp remote paths", () => {
|
||||
it("rejects remote attachment filenames with shell metacharacters before spawning scp", async () => {
|
||||
await withSandboxMediaTempHome("openclaw-triggers-", async (home) => {
|
||||
const { cfg, workspaceDir, sessionKey, remoteCacheDir } = createRemoteStageParams(home);
|
||||
const remotePath = "/Users/demo/Library/Messages/Attachments/ab/cd/evil$(touch pwned).jpg";
|
||||
const { ctx, sessionCtx } = createRemoteContexts(remotePath);
|
||||
|
||||
await stageSandboxMedia({
|
||||
ctx,
|
||||
sessionCtx,
|
||||
cfg,
|
||||
sessionKey,
|
||||
workspaceDir,
|
||||
});
|
||||
|
||||
expect(childProcessMocks.spawn).not.toHaveBeenCalled();
|
||||
await expect(fs.stat(join(remoteCacheDir, basename(remotePath)))).rejects.toThrow();
|
||||
expect(ctx.MediaPath).toBe(remotePath);
|
||||
expect(sessionCtx.MediaPath).toBe(remotePath);
|
||||
expect(ctx.MediaUrl).toBe(remotePath);
|
||||
expect(sessionCtx.MediaUrl).toBe(remotePath);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -7,7 +7,7 @@ import { ensureSandboxWorkspaceForSession } from "../../agents/sandbox.js";
|
|||
import type { OpenClawConfig } from "../../config/config.js";
|
||||
import { logVerbose } from "../../globals.js";
|
||||
import { copyFileWithinRoot, SafeOpenError } from "../../infra/fs-safe.js";
|
||||
import { normalizeScpRemoteHost } from "../../infra/scp-host.js";
|
||||
import { normalizeScpRemoteHost, normalizeScpRemotePath } from "../../infra/scp-host.js";
|
||||
import { resolvePreferredOpenClawTmpDir } from "../../infra/tmp-openclaw-dir.js";
|
||||
import {
|
||||
isInboundPathAllowed,
|
||||
|
|
@ -293,6 +293,10 @@ async function scpFile(remoteHost: string, remotePath: string, localPath: string
|
|||
if (!safeRemoteHost) {
|
||||
throw new Error("invalid remote host for SCP");
|
||||
}
|
||||
const safeRemotePath = normalizeScpRemotePath(remotePath);
|
||||
if (!safeRemotePath) {
|
||||
throw new Error("invalid remote path for SCP");
|
||||
}
|
||||
return new Promise((resolve, reject) => {
|
||||
const child = spawn(
|
||||
"/usr/bin/scp",
|
||||
|
|
@ -302,7 +306,7 @@ async function scpFile(remoteHost: string, remotePath: string, localPath: string
|
|||
"-o",
|
||||
"StrictHostKeyChecking=yes",
|
||||
"--",
|
||||
`${safeRemoteHost}:${remotePath}`,
|
||||
`${safeRemoteHost}:${safeRemotePath}`,
|
||||
localPath,
|
||||
],
|
||||
{ stdio: ["ignore", "ignore", "pipe"] },
|
||||
|
|
|
|||
|
|
@ -1,5 +1,10 @@
|
|||
import { describe, expect, it } from "vitest";
|
||||
import { isSafeScpRemoteHost, normalizeScpRemoteHost } from "./scp-host.js";
|
||||
import {
|
||||
isSafeScpRemoteHost,
|
||||
isSafeScpRemotePath,
|
||||
normalizeScpRemoteHost,
|
||||
normalizeScpRemotePath,
|
||||
} from "./scp-host.js";
|
||||
|
||||
describe("scp remote host", () => {
|
||||
it.each([
|
||||
|
|
@ -33,3 +38,40 @@ describe("scp remote host", () => {
|
|||
expect(isSafeScpRemoteHost(value)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("scp remote path", () => {
|
||||
it.each([
|
||||
{
|
||||
value: "/Users/demo/Library/Messages/Attachments/ab/cd/photo.jpg",
|
||||
expected: "/Users/demo/Library/Messages/Attachments/ab/cd/photo.jpg",
|
||||
},
|
||||
{
|
||||
value: " /Users/demo/Library/Messages/Attachments/ab/cd/IMG 1234 (1).jpg ",
|
||||
expected: "/Users/demo/Library/Messages/Attachments/ab/cd/IMG 1234 (1).jpg",
|
||||
},
|
||||
])("normalizes safe paths for %j", ({ value, expected }) => {
|
||||
expect(normalizeScpRemotePath(value)).toBe(expected);
|
||||
expect(isSafeScpRemotePath(value)).toBe(true);
|
||||
});
|
||||
|
||||
it.each([
|
||||
null,
|
||||
undefined,
|
||||
"",
|
||||
" ",
|
||||
"relative/path.jpg",
|
||||
"/Users/demo/Library/Messages/Attachments/ab/cd/bad$path.jpg",
|
||||
"/Users/demo/Library/Messages/Attachments/ab/cd/bad`path`.jpg",
|
||||
"/Users/demo/Library/Messages/Attachments/ab/cd/bad;path.jpg",
|
||||
"/Users/demo/Library/Messages/Attachments/ab/cd/bad|path.jpg",
|
||||
"/Users/demo/Library/Messages/Attachments/ab/cd/bad&path.jpg",
|
||||
"/Users/demo/Library/Messages/Attachments/ab/cd/bad<path.jpg",
|
||||
"/Users/demo/Library/Messages/Attachments/ab/cd/bad>path.jpg",
|
||||
'/Users/demo/Library/Messages/Attachments/ab/cd/bad"path.jpg',
|
||||
"/Users/demo/Library/Messages/Attachments/ab/cd/bad'path.jpg",
|
||||
"/Users/demo/Library/Messages/Attachments/ab/cd/bad\\path.jpg",
|
||||
])("rejects unsafe path tokens: %j", (value) => {
|
||||
expect(normalizeScpRemotePath(value)).toBeUndefined();
|
||||
expect(isSafeScpRemotePath(value)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
const SSH_TOKEN = /^[A-Za-z0-9._-]+$/;
|
||||
const BRACKETED_IPV6 = /^\[[0-9A-Fa-f:.%]+\]$/;
|
||||
const WHITESPACE = /\s/;
|
||||
const SCP_REMOTE_PATH_UNSAFE_CHARS = new Set(["\\", "'", '"', "`", "$", ";", "|", "&", "<", ">"]);
|
||||
|
||||
function hasControlOrWhitespace(value: string): boolean {
|
||||
for (const char of value) {
|
||||
|
|
@ -60,3 +61,26 @@ export function normalizeScpRemoteHost(value: string | null | undefined): string
|
|||
export function isSafeScpRemoteHost(value: string | null | undefined): boolean {
|
||||
return normalizeScpRemoteHost(value) !== undefined;
|
||||
}
|
||||
|
||||
export function normalizeScpRemotePath(value: string | null | undefined): string | undefined {
|
||||
if (typeof value !== "string") {
|
||||
return undefined;
|
||||
}
|
||||
const trimmed = value.trim();
|
||||
if (!trimmed || !trimmed.startsWith("/")) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
for (const char of trimmed) {
|
||||
const code = char.charCodeAt(0);
|
||||
if (code <= 0x1f || code === 0x7f || SCP_REMOTE_PATH_UNSAFE_CHARS.has(char)) {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
return trimmed;
|
||||
}
|
||||
|
||||
export function isSafeScpRemotePath(value: string | null | undefined): boolean {
|
||||
return normalizeScpRemotePath(value) !== undefined;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue