mirror of
https://github.com/openclaw/openclaw.git
synced 2026-02-19 18:39:20 -05:00
fix(security): harden safeBins stdin-only enforcement
This commit is contained in:
@@ -66,6 +66,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Sandbox/Registry: serialize container and browser registry writes with shared file locks and atomic replacement to prevent lost updates and delete rollback races from desyncing `sandbox list`, `prune`, and `recreate --all`. Thanks @kexinoh.
|
||||
- Security/Exec: require `tools.exec.safeBins` binaries to resolve from trusted bin directories (system defaults plus gateway startup `PATH`) so PATH-hijacked trojan binaries cannot bypass allowlist checks. Thanks @jackhax for reporting.
|
||||
- Security/Browser: route browser URL navigation through one SSRF-guarded validation path for tab-open/CDP-target/Playwright navigation flows and block private/metadata destinations by default (configurable via `browser.ssrfPolicy`). This ships in the next npm release. Thanks @dorjoos for reporting.
|
||||
- Security/Exec: for the next npm release, harden safe-bin stdin-only enforcement by blocking output/recursive flags (`sort -o/--output`, grep recursion) and tightening default safe bins to remove `sort`/`grep`, preventing safe-bin allowlist bypass for file writes/recursive reads. Thanks @nedlir for reporting.
|
||||
- Cron/Webhooks: protect cron webhook POST delivery with SSRF-guarded outbound fetch (`fetchWithSsrFGuard`) to block private/metadata destinations before request dispatch. Thanks @Adam55A-code.
|
||||
- Security/Net: block SSRF bypass via NAT64 (`64:ff9b::/96`, `64:ff9b:1::/48`), 6to4 (`2002::/16`), and Teredo (`2001:0000::/32`) IPv6 transition addresses, and fail closed on IPv6 parse errors. Thanks @jackhax.
|
||||
|
||||
|
||||
@@ -124,6 +124,8 @@ are treated as allowlisted on nodes (macOS node or headless node host). This use
|
||||
`tools.exec.safeBins` defines a small list of **stdin-only** binaries (for example `jq`)
|
||||
that can run in allowlist mode **without** explicit allowlist entries. Safe bins reject
|
||||
positional file args and path-like tokens, so they can only operate on the incoming stream.
|
||||
Safe bins also enforce explicit per-binary flag policy for options that break stdin-only
|
||||
behavior (for example `sort -o/--output` and grep recursive flags).
|
||||
Safe bins also force argv tokens to be treated as **literal text** at execution time (no globbing
|
||||
and no `$VARS` expansion) for stdin-only segments, so patterns like `*` or `$HOME/...` cannot be
|
||||
used to smuggle file reads.
|
||||
@@ -136,7 +138,10 @@ Shell chaining (`&&`, `||`, `;`) is allowed when every top-level segment satisfi
|
||||
Command substitution (`$()` / backticks) is rejected during allowlist parsing, including inside
|
||||
double quotes; use single quotes if you need literal `$()` text.
|
||||
|
||||
Default safe bins: `jq`, `grep`, `cut`, `sort`, `uniq`, `head`, `tail`, `tr`, `wc`.
|
||||
Default safe bins: `jq`, `cut`, `uniq`, `head`, `tail`, `tr`, `wc`.
|
||||
|
||||
`grep` and `sort` are not in the default list. If you opt in, keep explicit allowlist entries for
|
||||
their non-stdin workflows.
|
||||
|
||||
## Control UI editing
|
||||
|
||||
|
||||
@@ -157,4 +157,92 @@ describe("createOpenClawCodingTools safeBins", () => {
|
||||
expect(blockedResultDetails.status).toBe("completed");
|
||||
expect(text).not.toContain(secret);
|
||||
});
|
||||
|
||||
it("blocks sort output flags from writing files via safeBins", async () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
|
||||
const { createOpenClawCodingTools } = await import("./pi-tools.js");
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-safe-bins-sort-"));
|
||||
|
||||
const cfg: OpenClawConfig = {
|
||||
tools: {
|
||||
exec: {
|
||||
host: "gateway",
|
||||
security: "allowlist",
|
||||
ask: "off",
|
||||
safeBins: ["sort"],
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const tools = createOpenClawCodingTools({
|
||||
config: cfg,
|
||||
sessionKey: "agent:main:main",
|
||||
workspaceDir: tmpDir,
|
||||
agentDir: path.join(tmpDir, "agent"),
|
||||
});
|
||||
const execTool = tools.find((tool) => tool.name === "exec");
|
||||
expect(execTool).toBeDefined();
|
||||
|
||||
const shortTarget = path.join(tmpDir, "blocked-short.txt");
|
||||
await expect(
|
||||
execTool!.execute("call1", {
|
||||
command: "sort -oblocked-short.txt",
|
||||
workdir: tmpDir,
|
||||
}),
|
||||
).rejects.toThrow("exec denied: allowlist miss");
|
||||
expect(fs.existsSync(shortTarget)).toBe(false);
|
||||
|
||||
const longTarget = path.join(tmpDir, "blocked-long.txt");
|
||||
await expect(
|
||||
execTool!.execute("call2", {
|
||||
command: "sort --output=blocked-long.txt",
|
||||
workdir: tmpDir,
|
||||
}),
|
||||
).rejects.toThrow("exec denied: allowlist miss");
|
||||
expect(fs.existsSync(longTarget)).toBe(false);
|
||||
});
|
||||
|
||||
it("blocks grep recursive flags from reading cwd via safeBins", async () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
|
||||
const { createOpenClawCodingTools } = await import("./pi-tools.js");
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-safe-bins-grep-"));
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, "secret.txt"),
|
||||
"SAFE_BINS_RECURSIVE_SHOULD_NOT_LEAK\n",
|
||||
"utf8",
|
||||
);
|
||||
|
||||
const cfg: OpenClawConfig = {
|
||||
tools: {
|
||||
exec: {
|
||||
host: "gateway",
|
||||
security: "allowlist",
|
||||
ask: "off",
|
||||
safeBins: ["grep"],
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const tools = createOpenClawCodingTools({
|
||||
config: cfg,
|
||||
sessionKey: "agent:main:main",
|
||||
workspaceDir: tmpDir,
|
||||
agentDir: path.join(tmpDir, "agent"),
|
||||
});
|
||||
const execTool = tools.find((tool) => tool.name === "exec");
|
||||
expect(execTool).toBeDefined();
|
||||
|
||||
await expect(
|
||||
execTool!.execute("call1", {
|
||||
command: "grep -R SAFE_BINS_RECURSIVE_SHOULD_NOT_LEAK",
|
||||
workdir: tmpDir,
|
||||
}),
|
||||
).rejects.toThrow("exec denied: allowlist miss");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
import type { ExecAllowlistEntry } from "./exec-approvals.js";
|
||||
import {
|
||||
DEFAULT_SAFE_BINS,
|
||||
analyzeShellCommand,
|
||||
@@ -11,7 +12,6 @@ import {
|
||||
type CommandResolution,
|
||||
type ExecCommandSegment,
|
||||
} from "./exec-approvals-analysis.js";
|
||||
import type { ExecAllowlistEntry } from "./exec-approvals.js";
|
||||
import { isTrustedSafeBinPath } from "./exec-safe-bin-trust.js";
|
||||
|
||||
function isPathLikeToken(value: string): boolean {
|
||||
@@ -62,6 +62,57 @@ function hasGlobToken(value: string): boolean {
|
||||
return /[*?[\]]/.test(value);
|
||||
}
|
||||
|
||||
type SafeBinOptionPolicy = {
|
||||
blockedShort?: ReadonlySet<string>;
|
||||
blockedLong?: ReadonlySet<string>;
|
||||
};
|
||||
|
||||
const SAFE_BIN_OPTION_POLICIES: Readonly<Record<string, SafeBinOptionPolicy>> = {
|
||||
// sort can write arbitrary output paths via -o/--output, which breaks stdin-only guarantees.
|
||||
sort: {
|
||||
blockedShort: new Set(["o"]),
|
||||
blockedLong: new Set(["output"]),
|
||||
},
|
||||
// grep recursion flags read from cwd (or provided roots), so they are not stdin-only.
|
||||
grep: {
|
||||
blockedShort: new Set(["d", "r"]),
|
||||
blockedLong: new Set(["dereference-recursive", "directories", "recursive"]),
|
||||
},
|
||||
};
|
||||
|
||||
function parseLongOptionName(token: string): string | null {
|
||||
if (!token.startsWith("--") || token === "--") {
|
||||
return null;
|
||||
}
|
||||
const body = token.slice(2);
|
||||
if (!body) {
|
||||
return null;
|
||||
}
|
||||
const eqIndex = body.indexOf("=");
|
||||
const name = (eqIndex >= 0 ? body.slice(0, eqIndex) : body).trim().toLowerCase();
|
||||
return name.length > 0 ? name : null;
|
||||
}
|
||||
|
||||
function hasBlockedSafeBinOption(execName: string, token: string): boolean {
|
||||
const policy = SAFE_BIN_OPTION_POLICIES[execName];
|
||||
if (!policy || !token.startsWith("-")) {
|
||||
return false;
|
||||
}
|
||||
const longName = parseLongOptionName(token);
|
||||
if (longName) {
|
||||
return policy.blockedLong?.has(longName) ?? false;
|
||||
}
|
||||
if (token === "-" || token === "--") {
|
||||
return false;
|
||||
}
|
||||
for (const ch of token.slice(1)) {
|
||||
if (policy.blockedShort?.has(ch.toLowerCase())) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
export function isSafeBinUsage(params: {
|
||||
argv: string[];
|
||||
resolution: CommandResolution | null;
|
||||
@@ -112,6 +163,9 @@ export function isSafeBinUsage(params: {
|
||||
continue;
|
||||
}
|
||||
if (token.startsWith("-")) {
|
||||
if (hasBlockedSafeBinOption(execName, token)) {
|
||||
return false;
|
||||
}
|
||||
const eqIndex = token.indexOf("=");
|
||||
if (eqIndex > 0) {
|
||||
const value = token.slice(eqIndex + 1);
|
||||
|
||||
@@ -1,10 +1,10 @@
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
import { splitShellArgs } from "../utils/shell-argv.js";
|
||||
import type { ExecAllowlistEntry } from "./exec-approvals.js";
|
||||
import { splitShellArgs } from "../utils/shell-argv.js";
|
||||
import { expandHomePrefix } from "./home-dir.js";
|
||||
|
||||
export const DEFAULT_SAFE_BINS = ["jq", "grep", "cut", "sort", "uniq", "head", "tail", "tr", "wc"];
|
||||
export const DEFAULT_SAFE_BINS = ["jq", "cut", "uniq", "head", "tail", "tr", "wc"];
|
||||
|
||||
export type CommandResolution = {
|
||||
rawExecutable: string;
|
||||
|
||||
@@ -21,6 +21,7 @@ import {
|
||||
resolveExecApprovalsFromFile,
|
||||
resolveExecApprovalsPath,
|
||||
resolveExecApprovalsSocketPath,
|
||||
resolveSafeBins,
|
||||
type ExecAllowlistEntry,
|
||||
type ExecApprovalsFile,
|
||||
} from "./exec-approvals.js";
|
||||
@@ -414,6 +415,9 @@ describe("exec approvals safe bins", () => {
|
||||
argv: string[];
|
||||
resolvedPath: string;
|
||||
expected: boolean;
|
||||
safeBins?: string[];
|
||||
executableName?: string;
|
||||
rawExecutable?: string;
|
||||
cwd?: string;
|
||||
setup?: (cwd: string) => void;
|
||||
};
|
||||
@@ -439,6 +443,38 @@ describe("exec approvals safe bins", () => {
|
||||
expected: false,
|
||||
cwd: "/tmp",
|
||||
},
|
||||
{
|
||||
name: "blocks sort output path via -o <file>",
|
||||
argv: ["sort", "-o", "malicious.sh"],
|
||||
resolvedPath: "/usr/bin/sort",
|
||||
expected: false,
|
||||
safeBins: ["sort"],
|
||||
executableName: "sort",
|
||||
},
|
||||
{
|
||||
name: "blocks sort output path via attached short option (-ofile)",
|
||||
argv: ["sort", "-omalicious.sh"],
|
||||
resolvedPath: "/usr/bin/sort",
|
||||
expected: false,
|
||||
safeBins: ["sort"],
|
||||
executableName: "sort",
|
||||
},
|
||||
{
|
||||
name: "blocks sort output path via --output=file",
|
||||
argv: ["sort", "--output=malicious.sh"],
|
||||
resolvedPath: "/usr/bin/sort",
|
||||
expected: false,
|
||||
safeBins: ["sort"],
|
||||
executableName: "sort",
|
||||
},
|
||||
{
|
||||
name: "blocks grep recursive flags that read cwd",
|
||||
argv: ["grep", "-R", "needle"],
|
||||
resolvedPath: "/usr/bin/grep",
|
||||
expected: false,
|
||||
safeBins: ["grep"],
|
||||
executableName: "grep",
|
||||
},
|
||||
];
|
||||
|
||||
for (const testCase of cases) {
|
||||
@@ -448,14 +484,16 @@ describe("exec approvals safe bins", () => {
|
||||
}
|
||||
const cwd = testCase.cwd ?? makeTempDir();
|
||||
testCase.setup?.(cwd);
|
||||
const executableName = testCase.executableName ?? "jq";
|
||||
const rawExecutable = testCase.rawExecutable ?? executableName;
|
||||
const ok = isSafeBinUsage({
|
||||
argv: testCase.argv,
|
||||
resolution: {
|
||||
rawExecutable: "jq",
|
||||
rawExecutable,
|
||||
resolvedPath: testCase.resolvedPath,
|
||||
executableName: "jq",
|
||||
executableName,
|
||||
},
|
||||
safeBins: normalizeSafeBins(["jq"]),
|
||||
safeBins: normalizeSafeBins(testCase.safeBins ?? [executableName]),
|
||||
cwd,
|
||||
});
|
||||
expect(ok).toBe(testCase.expected);
|
||||
@@ -479,6 +517,13 @@ describe("exec approvals safe bins", () => {
|
||||
});
|
||||
expect(ok).toBe(true);
|
||||
});
|
||||
|
||||
it("does not include sort/grep in default safeBins", () => {
|
||||
const defaults = resolveSafeBins(undefined);
|
||||
expect(defaults.has("jq")).toBe(true);
|
||||
expect(defaults.has("sort")).toBe(false);
|
||||
expect(defaults.has("grep")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("exec approvals allowlist evaluation", () => {
|
||||
|
||||
Reference in New Issue
Block a user