mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-03 03:03:24 -04:00
fix(security): harden safeBins path trust
This commit is contained in:
@@ -10,6 +10,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
- Commands/Doctor: avoid rewriting invalid configs with new `gateway.auth.token` defaults during repair and only write when real config changes are detected, preventing accidental token duplication and backup churn.
|
||||
- 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.
|
||||
- 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.
|
||||
|
||||
|
||||
@@ -127,6 +127,8 @@ positional file args and path-like tokens, so they can only operate on the incom
|
||||
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.
|
||||
Safe bins must also resolve from trusted binary directories (system defaults plus the gateway
|
||||
process `PATH` at startup). This blocks request-scoped PATH hijacking attempts.
|
||||
Shell chaining and redirections are not auto-allowed in allowlist mode.
|
||||
|
||||
Shell chaining (`&&`, `||`, `;`) is allowed when every top-level segment satisfies the allowlist
|
||||
|
||||
@@ -51,7 +51,7 @@ Notes:
|
||||
- `tools.exec.ask` (default: `on-miss`)
|
||||
- `tools.exec.node` (default: unset)
|
||||
- `tools.exec.pathPrepend`: list of directories to prepend to `PATH` for exec runs (gateway + sandbox only).
|
||||
- `tools.exec.safeBins`: stdin-only safe binaries that can run without explicit allowlist entries.
|
||||
- `tools.exec.safeBins`: stdin-only safe binaries that can run without explicit allowlist entries (resolved path must come from trusted binary directories).
|
||||
|
||||
Example:
|
||||
|
||||
|
||||
@@ -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,48 @@ import {
|
||||
type CommandResolution,
|
||||
type ExecCommandSegment,
|
||||
} from "./exec-approvals-analysis.js";
|
||||
import type { ExecAllowlistEntry } from "./exec-approvals.js";
|
||||
|
||||
const DEFAULT_SAFE_BIN_TRUSTED_DIRS = [
|
||||
"/bin",
|
||||
"/usr/bin",
|
||||
"/usr/local/bin",
|
||||
"/opt/homebrew/bin",
|
||||
"/opt/local/bin",
|
||||
"/snap/bin",
|
||||
"/run/current-system/sw/bin",
|
||||
];
|
||||
|
||||
function normalizeTrustedDir(value: string): string | null {
|
||||
const trimmed = value.trim();
|
||||
if (!trimmed) {
|
||||
return null;
|
||||
}
|
||||
return path.resolve(trimmed);
|
||||
}
|
||||
|
||||
function collectTrustedSafeBinDirs(): Set<string> {
|
||||
const trusted = new Set<string>();
|
||||
for (const entry of DEFAULT_SAFE_BIN_TRUSTED_DIRS) {
|
||||
const normalized = normalizeTrustedDir(entry);
|
||||
if (normalized) {
|
||||
trusted.add(normalized);
|
||||
}
|
||||
}
|
||||
const pathEntries = (process.env.PATH ?? process.env.Path ?? "")
|
||||
.split(path.delimiter)
|
||||
.map((entry) => normalizeTrustedDir(entry))
|
||||
.filter((entry): entry is string => Boolean(entry));
|
||||
for (const entry of pathEntries) {
|
||||
trusted.add(entry);
|
||||
}
|
||||
return trusted;
|
||||
}
|
||||
|
||||
const TRUSTED_SAFE_BIN_DIRS = collectTrustedSafeBinDirs();
|
||||
|
||||
function isTrustedSafeBinPath(resolvedPath: string): boolean {
|
||||
return TRUSTED_SAFE_BIN_DIRS.has(path.dirname(path.resolve(resolvedPath)));
|
||||
}
|
||||
|
||||
function isPathLikeToken(value: string): boolean {
|
||||
const trimmed = value.trim();
|
||||
@@ -90,6 +132,9 @@ export function isSafeBinUsage(params: {
|
||||
if (!resolution?.resolvedPath) {
|
||||
return false;
|
||||
}
|
||||
if (!isTrustedSafeBinPath(resolution.resolvedPath)) {
|
||||
return false;
|
||||
}
|
||||
const cwd = params.cwd ?? process.cwd();
|
||||
const exists = params.fileExists ?? defaultFileExists;
|
||||
const argv = params.argv.slice(1);
|
||||
|
||||
@@ -34,26 +34,6 @@ function makeTempDir() {
|
||||
return fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-exec-approvals-"));
|
||||
}
|
||||
|
||||
function createSafeBinJqCase(params: { command: string; seedFileName?: string }) {
|
||||
const dir = makeTempDir();
|
||||
const binDir = path.join(dir, "bin");
|
||||
fs.mkdirSync(binDir, { recursive: true });
|
||||
const exeName = process.platform === "win32" ? "jq.exe" : "jq";
|
||||
const exe = path.join(binDir, exeName);
|
||||
fs.writeFileSync(exe, "");
|
||||
fs.chmodSync(exe, 0o755);
|
||||
if (params.seedFileName) {
|
||||
fs.writeFileSync(path.join(dir, params.seedFileName), "{}");
|
||||
}
|
||||
const res = analyzeShellCommand({
|
||||
command: params.command,
|
||||
cwd: dir,
|
||||
env: makePathEnv(binDir),
|
||||
});
|
||||
expect(res.ok).toBe(true);
|
||||
return { dir, segment: res.segments[0] };
|
||||
}
|
||||
|
||||
describe("exec approvals allowlist matching", () => {
|
||||
it("ignores basename-only patterns", () => {
|
||||
const resolution = {
|
||||
@@ -409,10 +389,14 @@ describe("exec approvals safe bins", () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
const { dir, segment } = createSafeBinJqCase({ command: "jq .foo" });
|
||||
const dir = makeTempDir();
|
||||
const ok = isSafeBinUsage({
|
||||
argv: segment.argv,
|
||||
resolution: segment.resolution,
|
||||
argv: ["jq", ".foo"],
|
||||
resolution: {
|
||||
rawExecutable: "jq",
|
||||
resolvedPath: "/usr/bin/jq",
|
||||
executableName: "jq",
|
||||
},
|
||||
safeBins: normalizeSafeBins(["jq"]),
|
||||
cwd: dir,
|
||||
});
|
||||
@@ -423,18 +407,37 @@ describe("exec approvals safe bins", () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
const { dir, segment } = createSafeBinJqCase({
|
||||
command: "jq .foo secret.json",
|
||||
seedFileName: "secret.json",
|
||||
});
|
||||
const dir = makeTempDir();
|
||||
fs.writeFileSync(path.join(dir, "secret.json"), "{}");
|
||||
const ok = isSafeBinUsage({
|
||||
argv: segment.argv,
|
||||
resolution: segment.resolution,
|
||||
argv: ["jq", ".foo", "secret.json"],
|
||||
resolution: {
|
||||
rawExecutable: "jq",
|
||||
resolvedPath: "/usr/bin/jq",
|
||||
executableName: "jq",
|
||||
},
|
||||
safeBins: normalizeSafeBins(["jq"]),
|
||||
cwd: dir,
|
||||
});
|
||||
expect(ok).toBe(false);
|
||||
});
|
||||
|
||||
it("blocks safe bins resolved from untrusted directories", () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
const ok = isSafeBinUsage({
|
||||
argv: ["jq", ".foo"],
|
||||
resolution: {
|
||||
rawExecutable: "jq",
|
||||
resolvedPath: "/tmp/evil-bin/jq",
|
||||
executableName: "jq",
|
||||
},
|
||||
safeBins: normalizeSafeBins(["jq"]),
|
||||
cwd: "/tmp",
|
||||
});
|
||||
expect(ok).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("exec approvals allowlist evaluation", () => {
|
||||
|
||||
Reference in New Issue
Block a user