fix(lobster): block arbitrary exec via lobsterPath/cwd (GHSA-4mhr-g7xj-cg8j) (#5335)
* fix(lobster): prevent arbitrary exec via lobsterPath/cwd * fix(lobster): harden lobsterPath errors + normalize cwd sandboxing * fix(lobster): ignore tool-provided lobsterPath; validate + use plugin config * fix(lobster): use plugin config lobsterPath + add tests (#5335) (thanks @vignesh07) * fix(lobster): make Windows spawn fallback handle ENOENT (#5335) (thanks @vignesh07) --------- Co-authored-by: Tyler Yust <TYTYYUST@YAHOO.COM>
This commit is contained in:
parent
34e2425b4d
commit
1295b67057
3 changed files with 226 additions and 45 deletions
|
|
@ -9,6 +9,7 @@ Docs: https://docs.openclaw.ai
|
||||||
### Fixes
|
### Fixes
|
||||||
|
|
||||||
- Telegram: restore draft streaming partials. (#5543) Thanks @obviyus.
|
- Telegram: restore draft streaming partials. (#5543) Thanks @obviyus.
|
||||||
|
- fix(lobster): block arbitrary exec via lobsterPath/cwd injection (GHSA-4mhr-g7xj-cg8j). (#5335) Thanks @vignesh07.
|
||||||
|
|
||||||
## 2026.1.30
|
## 2026.1.30
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -33,12 +33,13 @@ async function writeFakeLobster(params: { payload: unknown }) {
|
||||||
return await writeFakeLobsterScript(scriptBody);
|
return await writeFakeLobsterScript(scriptBody);
|
||||||
}
|
}
|
||||||
|
|
||||||
function fakeApi(): OpenClawPluginApi {
|
function fakeApi(overrides: Partial<OpenClawPluginApi> = {}): OpenClawPluginApi {
|
||||||
return {
|
return {
|
||||||
id: "lobster",
|
id: "lobster",
|
||||||
name: "lobster",
|
name: "lobster",
|
||||||
source: "test",
|
source: "test",
|
||||||
config: {} as any,
|
config: {} as any,
|
||||||
|
pluginConfig: {},
|
||||||
runtime: { version: "test" } as any,
|
runtime: { version: "test" } as any,
|
||||||
logger: { info() {}, warn() {}, error() {}, debug() {} },
|
logger: { info() {}, warn() {}, error() {}, debug() {} },
|
||||||
registerTool() {},
|
registerTool() {},
|
||||||
|
|
@ -48,7 +49,12 @@ function fakeApi(): OpenClawPluginApi {
|
||||||
registerCli() {},
|
registerCli() {},
|
||||||
registerService() {},
|
registerService() {},
|
||||||
registerProvider() {},
|
registerProvider() {},
|
||||||
|
registerHook() {},
|
||||||
|
registerHttpRoute() {},
|
||||||
|
registerCommand() {},
|
||||||
|
on() {},
|
||||||
resolvePath: (p) => p,
|
resolvePath: (p) => p,
|
||||||
|
...overrides,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -72,62 +78,159 @@ describe("lobster plugin tool", () => {
|
||||||
payload: { ok: true, status: "ok", output: [{ hello: "world" }], requiresApproval: null },
|
payload: { ok: true, status: "ok", output: [{ hello: "world" }], requiresApproval: null },
|
||||||
});
|
});
|
||||||
|
|
||||||
const tool = createLobsterTool(fakeApi());
|
const originalPath = process.env.PATH;
|
||||||
const res = await tool.execute("call1", {
|
process.env.PATH = `${fake.dir}${path.delimiter}${originalPath ?? ""}`;
|
||||||
action: "run",
|
|
||||||
pipeline: "noop",
|
|
||||||
lobsterPath: fake.binPath,
|
|
||||||
timeoutMs: 1000,
|
|
||||||
});
|
|
||||||
|
|
||||||
expect(res.details).toMatchObject({ ok: true, status: "ok" });
|
try {
|
||||||
|
const tool = createLobsterTool(fakeApi());
|
||||||
|
const res = await tool.execute("call1", {
|
||||||
|
action: "run",
|
||||||
|
pipeline: "noop",
|
||||||
|
timeoutMs: 1000,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(res.details).toMatchObject({ ok: true, status: "ok" });
|
||||||
|
} finally {
|
||||||
|
process.env.PATH = originalPath;
|
||||||
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
it("tolerates noisy stdout before the JSON envelope", async () => {
|
it("tolerates noisy stdout before the JSON envelope", async () => {
|
||||||
const payload = { ok: true, status: "ok", output: [], requiresApproval: null };
|
const payload = { ok: true, status: "ok", output: [], requiresApproval: null };
|
||||||
const { binPath } = await writeFakeLobsterScript(
|
const { dir } = await writeFakeLobsterScript(
|
||||||
`const payload = ${JSON.stringify(payload)};\n` +
|
`const payload = ${JSON.stringify(payload)};\n` +
|
||||||
`console.log("noise before json");\n` +
|
`console.log("noise before json");\n` +
|
||||||
`process.stdout.write(JSON.stringify(payload));\n`,
|
`process.stdout.write(JSON.stringify(payload));\n`,
|
||||||
"openclaw-lobster-plugin-noisy-",
|
"openclaw-lobster-plugin-noisy-",
|
||||||
);
|
);
|
||||||
|
|
||||||
const tool = createLobsterTool(fakeApi());
|
const originalPath = process.env.PATH;
|
||||||
const res = await tool.execute("call-noisy", {
|
process.env.PATH = `${dir}${path.delimiter}${originalPath ?? ""}`;
|
||||||
action: "run",
|
|
||||||
pipeline: "noop",
|
|
||||||
lobsterPath: binPath,
|
|
||||||
timeoutMs: 1000,
|
|
||||||
});
|
|
||||||
|
|
||||||
expect(res.details).toMatchObject({ ok: true, status: "ok" });
|
try {
|
||||||
});
|
const tool = createLobsterTool(fakeApi());
|
||||||
|
const res = await tool.execute("call-noisy", {
|
||||||
it("requires absolute lobsterPath when provided", async () => {
|
|
||||||
const tool = createLobsterTool(fakeApi());
|
|
||||||
await expect(
|
|
||||||
tool.execute("call2", {
|
|
||||||
action: "run",
|
action: "run",
|
||||||
pipeline: "noop",
|
pipeline: "noop",
|
||||||
lobsterPath: "./lobster",
|
timeoutMs: 1000,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(res.details).toMatchObject({ ok: true, status: "ok" });
|
||||||
|
} finally {
|
||||||
|
process.env.PATH = originalPath;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("requires absolute lobsterPath when provided (even though it is ignored)", async () => {
|
||||||
|
const fake = await writeFakeLobster({
|
||||||
|
payload: { ok: true, status: "ok", output: [{ hello: "world" }], requiresApproval: null },
|
||||||
|
});
|
||||||
|
|
||||||
|
const originalPath = process.env.PATH;
|
||||||
|
process.env.PATH = `${fake.dir}${path.delimiter}${originalPath ?? ""}`;
|
||||||
|
|
||||||
|
try {
|
||||||
|
const tool = createLobsterTool(fakeApi());
|
||||||
|
await expect(
|
||||||
|
tool.execute("call2", {
|
||||||
|
action: "run",
|
||||||
|
pipeline: "noop",
|
||||||
|
lobsterPath: "./lobster",
|
||||||
|
}),
|
||||||
|
).rejects.toThrow(/absolute path/);
|
||||||
|
} finally {
|
||||||
|
process.env.PATH = originalPath;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rejects lobsterPath (deprecated) when invalid", async () => {
|
||||||
|
const fake = await writeFakeLobster({
|
||||||
|
payload: { ok: true, status: "ok", output: [{ hello: "world" }], requiresApproval: null },
|
||||||
|
});
|
||||||
|
|
||||||
|
const originalPath = process.env.PATH;
|
||||||
|
process.env.PATH = `${fake.dir}${path.delimiter}${originalPath ?? ""}`;
|
||||||
|
|
||||||
|
try {
|
||||||
|
const tool = createLobsterTool(fakeApi());
|
||||||
|
await expect(
|
||||||
|
tool.execute("call2b", {
|
||||||
|
action: "run",
|
||||||
|
pipeline: "noop",
|
||||||
|
lobsterPath: "/bin/bash",
|
||||||
|
}),
|
||||||
|
).rejects.toThrow(/lobster executable/);
|
||||||
|
} finally {
|
||||||
|
process.env.PATH = originalPath;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rejects absolute cwd", async () => {
|
||||||
|
const tool = createLobsterTool(fakeApi());
|
||||||
|
await expect(
|
||||||
|
tool.execute("call2c", {
|
||||||
|
action: "run",
|
||||||
|
pipeline: "noop",
|
||||||
|
cwd: "/tmp",
|
||||||
}),
|
}),
|
||||||
).rejects.toThrow(/absolute path/);
|
).rejects.toThrow(/cwd must be a relative path/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rejects cwd that escapes the gateway working directory", async () => {
|
||||||
|
const tool = createLobsterTool(fakeApi());
|
||||||
|
await expect(
|
||||||
|
tool.execute("call2d", {
|
||||||
|
action: "run",
|
||||||
|
pipeline: "noop",
|
||||||
|
cwd: "../../etc",
|
||||||
|
}),
|
||||||
|
).rejects.toThrow(/must stay within/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("uses pluginConfig.lobsterPath when provided", async () => {
|
||||||
|
const fake = await writeFakeLobster({
|
||||||
|
payload: { ok: true, status: "ok", output: [{ hello: "world" }], requiresApproval: null },
|
||||||
|
});
|
||||||
|
|
||||||
|
// Ensure `lobster` is NOT discoverable via PATH, while still allowing our
|
||||||
|
// fake lobster (a Node script with `#!/usr/bin/env node`) to run.
|
||||||
|
const originalPath = process.env.PATH;
|
||||||
|
process.env.PATH = path.dirname(process.execPath);
|
||||||
|
|
||||||
|
try {
|
||||||
|
const tool = createLobsterTool(fakeApi({ pluginConfig: { lobsterPath: fake.binPath } }));
|
||||||
|
const res = await tool.execute("call-plugin-config", {
|
||||||
|
action: "run",
|
||||||
|
pipeline: "noop",
|
||||||
|
timeoutMs: 1000,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(res.details).toMatchObject({ ok: true, status: "ok" });
|
||||||
|
} finally {
|
||||||
|
process.env.PATH = originalPath;
|
||||||
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
it("rejects invalid JSON from lobster", async () => {
|
it("rejects invalid JSON from lobster", async () => {
|
||||||
const { binPath } = await writeFakeLobsterScript(
|
const { dir } = await writeFakeLobsterScript(
|
||||||
`process.stdout.write("nope");\n`,
|
`process.stdout.write("nope");\n`,
|
||||||
"openclaw-lobster-plugin-bad-",
|
"openclaw-lobster-plugin-bad-",
|
||||||
);
|
);
|
||||||
|
|
||||||
const tool = createLobsterTool(fakeApi());
|
const originalPath = process.env.PATH;
|
||||||
await expect(
|
process.env.PATH = `${dir}${path.delimiter}${originalPath ?? ""}`;
|
||||||
tool.execute("call3", {
|
|
||||||
action: "run",
|
try {
|
||||||
pipeline: "noop",
|
const tool = createLobsterTool(fakeApi());
|
||||||
lobsterPath: binPath,
|
await expect(
|
||||||
}),
|
tool.execute("call3", {
|
||||||
).rejects.toThrow(/invalid JSON/);
|
action: "run",
|
||||||
|
pipeline: "noop",
|
||||||
|
}),
|
||||||
|
).rejects.toThrow(/invalid JSON/);
|
||||||
|
} finally {
|
||||||
|
process.env.PATH = originalPath;
|
||||||
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
it("can be gated off in sandboxed contexts", async () => {
|
it("can be gated off in sandboxed contexts", async () => {
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,6 @@
|
||||||
import { Type } from "@sinclair/typebox";
|
import { Type } from "@sinclair/typebox";
|
||||||
import { spawn } from "node:child_process";
|
import { spawn } from "node:child_process";
|
||||||
|
import fs from "node:fs";
|
||||||
import path from "node:path";
|
import path from "node:path";
|
||||||
|
|
||||||
import type { OpenClawPluginApi } from "../../../src/plugins/types.js";
|
import type { OpenClawPluginApi } from "../../../src/plugins/types.js";
|
||||||
|
|
@ -23,18 +24,77 @@ type LobsterEnvelope =
|
||||||
|
|
||||||
function resolveExecutablePath(lobsterPathRaw: string | undefined) {
|
function resolveExecutablePath(lobsterPathRaw: string | undefined) {
|
||||||
const lobsterPath = lobsterPathRaw?.trim() || "lobster";
|
const lobsterPath = lobsterPathRaw?.trim() || "lobster";
|
||||||
if (lobsterPath !== "lobster" && !path.isAbsolute(lobsterPath)) {
|
|
||||||
throw new Error("lobsterPath must be an absolute path (or omit to use PATH)");
|
// SECURITY:
|
||||||
|
// Never allow arbitrary executables (e.g. /bin/bash). If the caller overrides
|
||||||
|
// the path, it must still be the lobster binary (by name) and be absolute.
|
||||||
|
if (lobsterPath !== "lobster") {
|
||||||
|
if (!path.isAbsolute(lobsterPath)) {
|
||||||
|
throw new Error("lobsterPath must be an absolute path (or omit to use PATH)");
|
||||||
|
}
|
||||||
|
const base = path.basename(lobsterPath).toLowerCase();
|
||||||
|
const allowed =
|
||||||
|
process.platform === "win32" ? ["lobster.exe", "lobster.cmd", "lobster.bat"] : ["lobster"];
|
||||||
|
if (!allowed.includes(base)) {
|
||||||
|
throw new Error("lobsterPath must point to the lobster executable");
|
||||||
|
}
|
||||||
|
let stat: fs.Stats;
|
||||||
|
try {
|
||||||
|
stat = fs.statSync(lobsterPath);
|
||||||
|
} catch {
|
||||||
|
throw new Error("lobsterPath must exist");
|
||||||
|
}
|
||||||
|
if (!stat.isFile()) {
|
||||||
|
throw new Error("lobsterPath must point to a file");
|
||||||
|
}
|
||||||
|
if (process.platform !== "win32") {
|
||||||
|
try {
|
||||||
|
fs.accessSync(lobsterPath, fs.constants.X_OK);
|
||||||
|
} catch {
|
||||||
|
throw new Error("lobsterPath must be executable");
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return lobsterPath;
|
return lobsterPath;
|
||||||
}
|
}
|
||||||
|
|
||||||
function isWindowsSpawnEINVAL(err: unknown) {
|
function normalizeForCwdSandbox(p: string): string {
|
||||||
|
const normalized = path.normalize(p);
|
||||||
|
return process.platform === "win32" ? normalized.toLowerCase() : normalized;
|
||||||
|
}
|
||||||
|
|
||||||
|
function resolveCwd(cwdRaw: unknown): string {
|
||||||
|
if (typeof cwdRaw !== "string" || !cwdRaw.trim()) {
|
||||||
|
return process.cwd();
|
||||||
|
}
|
||||||
|
const cwd = cwdRaw.trim();
|
||||||
|
if (path.isAbsolute(cwd)) {
|
||||||
|
throw new Error("cwd must be a relative path");
|
||||||
|
}
|
||||||
|
const base = process.cwd();
|
||||||
|
const resolved = path.resolve(base, cwd);
|
||||||
|
|
||||||
|
const rel = path.relative(normalizeForCwdSandbox(base), normalizeForCwdSandbox(resolved));
|
||||||
|
if (rel === "" || rel === ".") {
|
||||||
|
return resolved;
|
||||||
|
}
|
||||||
|
if (rel.startsWith("..") || path.isAbsolute(rel)) {
|
||||||
|
throw new Error("cwd must stay within the gateway working directory");
|
||||||
|
}
|
||||||
|
return resolved;
|
||||||
|
}
|
||||||
|
|
||||||
|
function isWindowsSpawnErrorThatCanUseShell(err: unknown) {
|
||||||
if (!err || typeof err !== "object") {
|
if (!err || typeof err !== "object") {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
const code = (err as { code?: unknown }).code;
|
const code = (err as { code?: unknown }).code;
|
||||||
return code === "EINVAL";
|
|
||||||
|
// On Windows, spawning scripts discovered on PATH (e.g. lobster.cmd) can fail
|
||||||
|
// with EINVAL, and PATH discovery itself can fail with ENOENT when the binary
|
||||||
|
// is only available via PATHEXT/script wrappers.
|
||||||
|
return code === "EINVAL" || code === "ENOENT";
|
||||||
}
|
}
|
||||||
|
|
||||||
async function runLobsterSubprocessOnce(
|
async function runLobsterSubprocessOnce(
|
||||||
|
|
@ -125,7 +185,7 @@ async function runLobsterSubprocess(params: {
|
||||||
try {
|
try {
|
||||||
return await runLobsterSubprocessOnce(params, false);
|
return await runLobsterSubprocessOnce(params, false);
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
if (process.platform === "win32" && isWindowsSpawnEINVAL(err)) {
|
if (process.platform === "win32" && isWindowsSpawnErrorThatCanUseShell(err)) {
|
||||||
return await runLobsterSubprocessOnce(params, true);
|
return await runLobsterSubprocessOnce(params, true);
|
||||||
}
|
}
|
||||||
throw err;
|
throw err;
|
||||||
|
|
@ -182,8 +242,17 @@ export function createLobsterTool(api: OpenClawPluginApi) {
|
||||||
argsJson: Type.Optional(Type.String()),
|
argsJson: Type.Optional(Type.String()),
|
||||||
token: Type.Optional(Type.String()),
|
token: Type.Optional(Type.String()),
|
||||||
approve: Type.Optional(Type.Boolean()),
|
approve: Type.Optional(Type.Boolean()),
|
||||||
lobsterPath: Type.Optional(Type.String()),
|
// SECURITY: Do not allow the agent to choose an executable path.
|
||||||
cwd: Type.Optional(Type.String()),
|
// Host can configure the lobster binary via plugin config.
|
||||||
|
lobsterPath: Type.Optional(
|
||||||
|
Type.String({ description: "(deprecated) Use plugin config instead." }),
|
||||||
|
),
|
||||||
|
cwd: Type.Optional(
|
||||||
|
Type.String({
|
||||||
|
description:
|
||||||
|
"Relative working directory (optional). Must stay within the gateway working directory.",
|
||||||
|
}),
|
||||||
|
),
|
||||||
timeoutMs: Type.Optional(Type.Number()),
|
timeoutMs: Type.Optional(Type.Number()),
|
||||||
maxStdoutBytes: Type.Optional(Type.Number()),
|
maxStdoutBytes: Type.Optional(Type.Number()),
|
||||||
}),
|
}),
|
||||||
|
|
@ -193,11 +262,19 @@ export function createLobsterTool(api: OpenClawPluginApi) {
|
||||||
throw new Error("action required");
|
throw new Error("action required");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// SECURITY: never allow tool callers (agent/user) to select executables.
|
||||||
|
// If a host needs to override the binary, it must do so via plugin config.
|
||||||
|
// We still validate the parameter shape to prevent reintroducing an RCE footgun.
|
||||||
|
if (typeof params.lobsterPath === "string" && params.lobsterPath.trim()) {
|
||||||
|
resolveExecutablePath(params.lobsterPath);
|
||||||
|
}
|
||||||
|
|
||||||
const execPath = resolveExecutablePath(
|
const execPath = resolveExecutablePath(
|
||||||
typeof params.lobsterPath === "string" ? params.lobsterPath : undefined,
|
typeof api.pluginConfig?.lobsterPath === "string"
|
||||||
|
? api.pluginConfig.lobsterPath
|
||||||
|
: undefined,
|
||||||
);
|
);
|
||||||
const cwd =
|
const cwd = resolveCwd(params.cwd);
|
||||||
typeof params.cwd === "string" && params.cwd.trim() ? params.cwd.trim() : process.cwd();
|
|
||||||
const timeoutMs = typeof params.timeoutMs === "number" ? params.timeoutMs : 20_000;
|
const timeoutMs = typeof params.timeoutMs === "number" ? params.timeoutMs : 20_000;
|
||||||
const maxStdoutBytes =
|
const maxStdoutBytes =
|
||||||
typeof params.maxStdoutBytes === "number" ? params.maxStdoutBytes : 512_000;
|
typeof params.maxStdoutBytes === "number" ? params.maxStdoutBytes : 512_000;
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue