From 666f48d9b882a8a1415ca53f9567c72499d850c9 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Gondhi Date: Mon, 13 Apr 2026 12:03:15 +0530 Subject: [PATCH] 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 --- CHANGELOG.md | 1 + .../doctor/shared/exec-safe-bins.test.ts | 17 ++++++ .../exec-safe-bin-runtime-policy.test.ts | 4 +- src/node-host/invoke-system-run-plan.test.ts | 60 +++++++++++++++++++ src/node-host/invoke-system-run-plan.ts | 28 +++++++-- 5 files changed, 105 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c02fad47ef7..3314c8d0991 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. diff --git a/src/commands/doctor/shared/exec-safe-bins.test.ts b/src/commands/doctor/shared/exec-safe-bins.test.ts index 2e08326b924..bff3ef2452c 100644 --- a/src/commands/doctor/shared/exec-safe-bins.test.ts +++ b/src/commands/doctor/shared/exec-safe-bins.test.ts @@ -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"); diff --git a/src/infra/exec-safe-bin-runtime-policy.test.ts b/src/infra/exec-safe-bin-runtime-policy.test.ts index 5968bd6ef9b..558f432676a 100644 --- a/src/infra/exec-safe-bin-runtime-policy.test.ts +++ b/src/infra/exec-safe-bin-runtime-policy.test.ts @@ -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", () => { diff --git a/src/node-host/invoke-system-run-plan.test.ts b/src/node-host/invoke-system-run-plan.test.ts index d81c0e60aba..c4be8f593e3 100644 --- a/src/node-host/invoke-system-run-plan.test.ts +++ b/src/node-host/invoke-system-run-plan.test.ts @@ -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", diff --git a/src/node-host/invoke-system-run-plan.ts b/src/node-host/invoke-system-run-plan.ts index 541eb2b5a18..be13a088d88 100644 --- a/src/node-host/invoke-system-run-plan.ts +++ b/src/node-host/invoke-system-run-plan.ts @@ -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).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;