mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 15:40:44 +00:00
fix(security): remove busybox/toybox from interpreter-like safe bins [AI-assisted] (#65713)
* fix: address issue * fix: address review feedback * fix: address PR review feedback * fix: address review-pr skill feedback * fix: address PR review feedback * docs: add changelog entry for PR merge
This commit is contained in:
committed by
GitHub
parent
0a105c0900
commit
666f48d9b8
@@ -10,6 +10,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- fix(security): remove busybox/toybox from interpreter-like safe bins [AI-assisted]. (#65713) Thanks @pgondhi987.
|
||||
- fix(approval-auth): prevent empty approver list from granting explicit approval authorization [AI]. (#65714) Thanks @pgondhi987.
|
||||
- fix(security): broaden shell-wrapper detection and block env-argv assignment injection [AI-assisted]. (#65717) Thanks @pgondhi987.
|
||||
- Gateway/startup: defer scheduled services until sidecars finish, gate chat history and model listing during sidecar resume, and let Control UI retry startup-gated history loads so Sandbox wake resumes channels first. (#65365) Thanks @lml2468.
|
||||
|
||||
@@ -100,6 +100,23 @@ describe("doctor exec safe bin helpers", () => {
|
||||
expect(result.config.tools?.exec?.safeBinProfiles).toEqual({});
|
||||
});
|
||||
|
||||
it("warns on busybox/toybox safeBins instead of scaffolding them", () => {
|
||||
const result = maybeRepairExecSafeBinProfiles({
|
||||
tools: {
|
||||
exec: {
|
||||
safeBins: ["busybox", "toybox"],
|
||||
},
|
||||
},
|
||||
} as OpenClawConfig);
|
||||
|
||||
expect(result.changes).toEqual([]);
|
||||
expect(result.warnings).toEqual([
|
||||
"- tools.exec.safeBins includes interpreter/runtime 'busybox' without profile; remove it from safeBins or use explicit allowlist entries.",
|
||||
"- tools.exec.safeBins includes interpreter/runtime 'toybox' without profile; remove it from safeBins or use explicit allowlist entries.",
|
||||
]);
|
||||
expect(result.config.tools?.exec?.safeBinProfiles).toEqual({});
|
||||
});
|
||||
|
||||
it("flags safeBins that resolve outside trusted directories", () => {
|
||||
const tempDir = mkdtempSync(join(tmpdir(), "openclaw-safe-bin-"));
|
||||
const binPath = join(tempDir, "custom-safe-bin");
|
||||
|
||||
@@ -43,10 +43,12 @@ describe("exec safe-bin runtime policy", () => {
|
||||
"jq",
|
||||
" C:\\Tools\\Python3.EXE ",
|
||||
"myfilter",
|
||||
"busybox",
|
||||
"toybox",
|
||||
"/usr/bin/node",
|
||||
"/opt/homebrew/bin/gawk",
|
||||
]),
|
||||
).toEqual(["gawk", "node", "python3"]);
|
||||
).toEqual(["busybox", "gawk", "node", "python3", "toybox"]);
|
||||
});
|
||||
|
||||
it("merges and normalizes safe-bin profile fixtures", () => {
|
||||
|
||||
@@ -202,6 +202,66 @@ const unsafeRuntimeInvocationCases: UnsafeRuntimeInvocationCase[] = [
|
||||
tmpPrefix: "openclaw-tsx-eval-",
|
||||
command: ["tsx", "--eval", "console.log('SAFE')"],
|
||||
},
|
||||
{
|
||||
name: "rejects busybox applets that cannot be safely bound",
|
||||
binName: "busybox",
|
||||
tmpPrefix: "openclaw-busybox-awk-",
|
||||
command: ["busybox", "awk", 'BEGIN{system("id")}'],
|
||||
},
|
||||
{
|
||||
name: "rejects busybox applets even when cwd contains a file named after the applet",
|
||||
binName: "busybox",
|
||||
tmpPrefix: "openclaw-busybox-awk-file-bait-",
|
||||
command: ["busybox", "awk", 'BEGIN{system("id")}'],
|
||||
setup: (tmp) => {
|
||||
fs.writeFileSync(path.join(tmp, "awk"), "bait\n");
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "rejects busybox shell applets that forward inline commands",
|
||||
binName: "busybox",
|
||||
tmpPrefix: "openclaw-busybox-shell-inline-",
|
||||
command: ["busybox", "sh", "-c", "echo SAFE"],
|
||||
},
|
||||
{
|
||||
name: "rejects busybox shell applets with script file operands",
|
||||
binName: "busybox",
|
||||
tmpPrefix: "openclaw-busybox-shell-file-",
|
||||
command: ["busybox", "sh", "./run.sh"],
|
||||
setup: (tmp) => {
|
||||
fs.writeFileSync(path.join(tmp, "run.sh"), "#!/bin/sh\necho SAFE\n");
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "rejects toybox applets that cannot be safely bound",
|
||||
binName: "toybox",
|
||||
tmpPrefix: "openclaw-toybox-awk-",
|
||||
command: ["toybox", "awk", 'BEGIN{system("id")}'],
|
||||
},
|
||||
{
|
||||
name: "rejects toybox applets even when cwd contains a file named after the applet",
|
||||
binName: "toybox",
|
||||
tmpPrefix: "openclaw-toybox-awk-file-bait-",
|
||||
command: ["toybox", "awk", 'BEGIN{system("id")}'],
|
||||
setup: (tmp) => {
|
||||
fs.writeFileSync(path.join(tmp, "awk"), "bait\n");
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "rejects toybox shell applets that forward inline commands",
|
||||
binName: "toybox",
|
||||
tmpPrefix: "openclaw-toybox-shell-inline-",
|
||||
command: ["toybox", "ash", "-lc", "echo SAFE"],
|
||||
},
|
||||
{
|
||||
name: "rejects toybox shell applets with script file operands",
|
||||
binName: "toybox",
|
||||
tmpPrefix: "openclaw-toybox-shell-file-",
|
||||
command: ["toybox", "ash", "./run.sh"],
|
||||
setup: (tmp) => {
|
||||
fs.writeFileSync(path.join(tmp, "run.sh"), "#!/bin/sh\necho SAFE\n");
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "rejects node inline import operands that cannot be bound to one stable file",
|
||||
binName: "node",
|
||||
|
||||
@@ -47,6 +47,8 @@ const GENERIC_MUTABLE_SCRIPT_RUNNERS = new Set([
|
||||
"vite-node",
|
||||
]);
|
||||
|
||||
const OPAQUE_MUTABLE_SCRIPT_RUNNERS = new Set(["busybox", "toybox"]);
|
||||
|
||||
const BUN_SUBCOMMANDS = new Set([
|
||||
"add",
|
||||
"audit",
|
||||
@@ -283,9 +285,14 @@ function resolvesToExistingFileSync(rawOperand: string, cwd: string | undefined)
|
||||
}
|
||||
}
|
||||
|
||||
function unwrapArgvForMutableOperand(argv: string[]): { argv: string[]; baseIndex: number } {
|
||||
function unwrapArgvForMutableOperand(argv: string[]): {
|
||||
argv: string[];
|
||||
baseIndex: number;
|
||||
opaqueMultiplexerSeen: boolean;
|
||||
} {
|
||||
let current = argv;
|
||||
let baseIndex = 0;
|
||||
let opaqueMultiplexerSeen = false;
|
||||
while (true) {
|
||||
const dispatchUnwrap = unwrapKnownDispatchWrapperInvocation(current);
|
||||
if (dispatchUnwrap.kind === "unwrapped") {
|
||||
@@ -295,6 +302,9 @@ function unwrapArgvForMutableOperand(argv: string[]): { argv: string[]; baseInde
|
||||
}
|
||||
const shellMultiplexerUnwrap = unwrapKnownShellMultiplexerInvocation(current);
|
||||
if (shellMultiplexerUnwrap.kind === "unwrapped") {
|
||||
if (OPAQUE_MUTABLE_SCRIPT_RUNNERS.has(shellMultiplexerUnwrap.wrapper)) {
|
||||
opaqueMultiplexerSeen = true;
|
||||
}
|
||||
baseIndex += current.length - shellMultiplexerUnwrap.argv.length;
|
||||
current = shellMultiplexerUnwrap.argv;
|
||||
continue;
|
||||
@@ -305,7 +315,7 @@ function unwrapArgvForMutableOperand(argv: string[]): { argv: string[]; baseInde
|
||||
current = packageManagerUnwrap;
|
||||
continue;
|
||||
}
|
||||
return { argv: current, baseIndex };
|
||||
return { argv: current, baseIndex, opaqueMultiplexerSeen };
|
||||
}
|
||||
}
|
||||
|
||||
@@ -743,7 +753,11 @@ function hasPerlUnsafeApprovalFlag(argv: string[]): boolean {
|
||||
}
|
||||
|
||||
function isMutableScriptRunner(executable: string): boolean {
|
||||
return GENERIC_MUTABLE_SCRIPT_RUNNERS.has(executable) || isInterpreterLikeSafeBin(executable);
|
||||
return (
|
||||
GENERIC_MUTABLE_SCRIPT_RUNNERS.has(executable) ||
|
||||
OPAQUE_MUTABLE_SCRIPT_RUNNERS.has(executable) ||
|
||||
isInterpreterLikeSafeBin(executable)
|
||||
);
|
||||
}
|
||||
|
||||
function resolveMutableFileOperandIndex(argv: string[], cwd: string | undefined): number | null {
|
||||
@@ -752,6 +766,9 @@ function resolveMutableFileOperandIndex(argv: string[], cwd: string | undefined)
|
||||
if (!executable) {
|
||||
return null;
|
||||
}
|
||||
if (unwrapped.opaqueMultiplexerSeen || OPAQUE_MUTABLE_SCRIPT_RUNNERS.has(executable)) {
|
||||
return null;
|
||||
}
|
||||
if ((POSIX_SHELL_WRAPPERS as ReadonlySet<string>).has(executable)) {
|
||||
const shellIndex = resolvePosixShellScriptOperandIndex(unwrapped.argv);
|
||||
return shellIndex === null ? null : unwrapped.baseIndex + shellIndex;
|
||||
@@ -823,13 +840,16 @@ function requiresStableInterpreterApprovalBindingWithShellCommand(params: {
|
||||
shellCommand: string | null;
|
||||
cwd: string | undefined;
|
||||
}): boolean {
|
||||
const unwrapped = unwrapArgvForMutableOperand(params.argv);
|
||||
if (unwrapped.opaqueMultiplexerSeen) {
|
||||
return true;
|
||||
}
|
||||
if (params.shellCommand !== null) {
|
||||
return shellPayloadNeedsStableBinding(params.shellCommand, params.cwd);
|
||||
}
|
||||
if (pnpmDlxInvocationNeedsFailClosedBinding(params.argv, params.cwd)) {
|
||||
return true;
|
||||
}
|
||||
const unwrapped = unwrapArgvForMutableOperand(params.argv);
|
||||
const executable = normalizeExecutableToken(unwrapped.argv[0] ?? "");
|
||||
if (!executable) {
|
||||
return false;
|
||||
|
||||
Reference in New Issue
Block a user