Skip to content

Commit 666f48d

Browse files
pgondhi987pgondhi987
authored
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
1 parent 0a105c0 commit 666f48d

5 files changed

Lines changed: 105 additions & 5 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ Docs: https://docs.openclaw.ai
1010

1111
### Fixes
1212

13+
- fix(security): remove busybox/toybox from interpreter-like safe bins [AI-assisted]. (#65713) Thanks @pgondhi987.
1314
- fix(approval-auth): prevent empty approver list from granting explicit approval authorization [AI]. (#65714) Thanks @pgondhi987.
1415
- fix(security): broaden shell-wrapper detection and block env-argv assignment injection [AI-assisted]. (#65717) Thanks @pgondhi987.
1516
- 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.

‎src/commands/doctor/shared/exec-safe-bins.test.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,23 @@ describe("doctor exec safe bin helpers", () => {
100100
expect(result.config.tools?.exec?.safeBinProfiles).toEqual({});
101101
});
102102

103+
it("warns on busybox/toybox safeBins instead of scaffolding them", () => {
104+
const result = maybeRepairExecSafeBinProfiles({
105+
tools: {
106+
exec: {
107+
safeBins: ["busybox", "toybox"],
108+
},
109+
},
110+
} as OpenClawConfig);
111+
112+
expect(result.changes).toEqual([]);
113+
expect(result.warnings).toEqual([
114+
"- tools.exec.safeBins includes interpreter/runtime 'busybox' without profile; remove it from safeBins or use explicit allowlist entries.",
115+
"- tools.exec.safeBins includes interpreter/runtime 'toybox' without profile; remove it from safeBins or use explicit allowlist entries.",
116+
]);
117+
expect(result.config.tools?.exec?.safeBinProfiles).toEqual({});
118+
});
119+
103120
it("flags safeBins that resolve outside trusted directories", () => {
104121
const tempDir = mkdtempSync(join(tmpdir(), "openclaw-safe-bin-"));
105122
const binPath = join(tempDir, "custom-safe-bin");

‎src/infra/exec-safe-bin-runtime-policy.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,12 @@ describe("exec safe-bin runtime policy", () => {
4343
"jq",
4444
" C:\\Tools\\Python3.EXE ",
4545
"myfilter",
46+
"busybox",
47+
"toybox",
4648
"/usr/bin/node",
4749
"/opt/homebrew/bin/gawk",
4850
]),
49-
).toEqual(["gawk", "node", "python3"]);
51+
).toEqual(["busybox", "gawk", "node", "python3", "toybox"]);
5052
});
5153

5254
it("merges and normalizes safe-bin profile fixtures", () => {

‎src/node-host/invoke-system-run-plan.test.ts‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,66 @@ const unsafeRuntimeInvocationCases: UnsafeRuntimeInvocationCase[] = [
202202
tmpPrefix: "openclaw-tsx-eval-",
203203
command: ["tsx", "--eval", "console.log('SAFE')"],
204204
},
205+
{
206+
name: "rejects busybox applets that cannot be safely bound",
207+
binName: "busybox",
208+
tmpPrefix: "openclaw-busybox-awk-",
209+
command: ["busybox", "awk", 'BEGIN{system("id")}'],
210+
},
211+
{
212+
name: "rejects busybox applets even when cwd contains a file named after the applet",
213+
binName: "busybox",
214+
tmpPrefix: "openclaw-busybox-awk-file-bait-",
215+
command: ["busybox", "awk", 'BEGIN{system("id")}'],
216+
setup: (tmp) => {
217+
fs.writeFileSync(path.join(tmp, "awk"), "bait\n");
218+
},
219+
},
220+
{
221+
name: "rejects busybox shell applets that forward inline commands",
222+
binName: "busybox",
223+
tmpPrefix: "openclaw-busybox-shell-inline-",
224+
command: ["busybox", "sh", "-c", "echo SAFE"],
225+
},
226+
{
227+
name: "rejects busybox shell applets with script file operands",
228+
binName: "busybox",
229+
tmpPrefix: "openclaw-busybox-shell-file-",
230+
command: ["busybox", "sh", "./run.sh"],
231+
setup: (tmp) => {
232+
fs.writeFileSync(path.join(tmp, "run.sh"), "#!/bin/sh\necho SAFE\n");
233+
},
234+
},
235+
{
236+
name: "rejects toybox applets that cannot be safely bound",
237+
binName: "toybox",
238+
tmpPrefix: "openclaw-toybox-awk-",
239+
command: ["toybox", "awk", 'BEGIN{system("id")}'],
240+
},
241+
{
242+
name: "rejects toybox applets even when cwd contains a file named after the applet",
243+
binName: "toybox",
244+
tmpPrefix: "openclaw-toybox-awk-file-bait-",
245+
command: ["toybox", "awk", 'BEGIN{system("id")}'],
246+
setup: (tmp) => {
247+
fs.writeFileSync(path.join(tmp, "awk"), "bait\n");
248+
},
249+
},
250+
{
251+
name: "rejects toybox shell applets that forward inline commands",
252+
binName: "toybox",
253+
tmpPrefix: "openclaw-toybox-shell-inline-",
254+
command: ["toybox", "ash", "-lc", "echo SAFE"],
255+
},
256+
{
257+
name: "rejects toybox shell applets with script file operands",
258+
binName: "toybox",
259+
tmpPrefix: "openclaw-toybox-shell-file-",
260+
command: ["toybox", "ash", "./run.sh"],
261+
setup: (tmp) => {
262+
fs.writeFileSync(path.join(tmp, "run.sh"), "#!/bin/sh\necho SAFE\n");
263+
},
264+
},
205265
{
206266
name: "rejects node inline import operands that cannot be bound to one stable file",
207267
binName: "node",

‎src/node-host/invoke-system-run-plan.ts‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ const GENERIC_MUTABLE_SCRIPT_RUNNERS = new Set([
4747
"vite-node",
4848
]);
4949

50+
const OPAQUE_MUTABLE_SCRIPT_RUNNERS = new Set(["busybox", "toybox"]);
51+
5052
const BUN_SUBCOMMANDS = new Set([
5153
"add",
5254
"audit",
@@ -283,9 +285,14 @@ function resolvesToExistingFileSync(rawOperand: string, cwd: string | undefined)
283285
}
284286
}
285287

286-
function unwrapArgvForMutableOperand(argv: string[]): { argv: string[]; baseIndex: number } {
288+
function unwrapArgvForMutableOperand(argv: string[]): {
289+
argv: string[];
290+
baseIndex: number;
291+
opaqueMultiplexerSeen: boolean;
292+
} {
287293
let current = argv;
288294
let baseIndex = 0;
295+
let opaqueMultiplexerSeen = false;
289296
while (true) {
290297
const dispatchUnwrap = unwrapKnownDispatchWrapperInvocation(current);
291298
if (dispatchUnwrap.kind === "unwrapped") {
@@ -295,6 +302,9 @@ function unwrapArgvForMutableOperand(argv: string[]): { argv: string[]; baseInde
295302
}
296303
const shellMultiplexerUnwrap = unwrapKnownShellMultiplexerInvocation(current);
297304
if (shellMultiplexerUnwrap.kind === "unwrapped") {
305+
if (OPAQUE_MUTABLE_SCRIPT_RUNNERS.has(shellMultiplexerUnwrap.wrapper)) {
306+
opaqueMultiplexerSeen = true;
307+
}
298308
baseIndex += current.length - shellMultiplexerUnwrap.argv.length;
299309
current = shellMultiplexerUnwrap.argv;
300310
continue;
@@ -305,7 +315,7 @@ function unwrapArgvForMutableOperand(argv: string[]): { argv: string[]; baseInde
305315
current = packageManagerUnwrap;
306316
continue;
307317
}
308-
return { argv: current, baseIndex };
318+
return { argv: current, baseIndex, opaqueMultiplexerSeen };
309319
}
310320
}
311321

@@ -743,7 +753,11 @@ function hasPerlUnsafeApprovalFlag(argv: string[]): boolean {
743753
}
744754

745755
function isMutableScriptRunner(executable: string): boolean {
746-
return GENERIC_MUTABLE_SCRIPT_RUNNERS.has(executable) || isInterpreterLikeSafeBin(executable);
756+
return (
757+
GENERIC_MUTABLE_SCRIPT_RUNNERS.has(executable) ||
758+
OPAQUE_MUTABLE_SCRIPT_RUNNERS.has(executable) ||
759+
isInterpreterLikeSafeBin(executable)
760+
);
747761
}
748762

749763
function resolveMutableFileOperandIndex(argv: string[], cwd: string | undefined): number | null {
@@ -752,6 +766,9 @@ function resolveMutableFileOperandIndex(argv: string[], cwd: string | undefined)
752766
if (!executable) {
753767
return null;
754768
}
769+
if (unwrapped.opaqueMultiplexerSeen || OPAQUE_MUTABLE_SCRIPT_RUNNERS.has(executable)) {
770+
return null;
771+
}
755772
if ((POSIX_SHELL_WRAPPERS as ReadonlySet<string>).has(executable)) {
756773
const shellIndex = resolvePosixShellScriptOperandIndex(unwrapped.argv);
757774
return shellIndex === null ? null : unwrapped.baseIndex + shellIndex;
@@ -823,13 +840,16 @@ function requiresStableInterpreterApprovalBindingWithShellCommand(params: {
823840
shellCommand: string | null;
824841
cwd: string | undefined;
825842
}): boolean {
843+
const unwrapped = unwrapArgvForMutableOperand(params.argv);
844+
if (unwrapped.opaqueMultiplexerSeen) {
845+
return true;
846+
}
826847
if (params.shellCommand !== null) {
827848
return shellPayloadNeedsStableBinding(params.shellCommand, params.cwd);
828849
}
829850
if (pnpmDlxInvocationNeedsFailClosedBinding(params.argv, params.cwd)) {
830851
return true;
831852
}
832-
const unwrapped = unwrapArgvForMutableOperand(params.argv);
833853
const executable = normalizeExecutableToken(unwrapped.argv[0] ?? "");
834854
if (!executable) {
835855
return false;

0 commit comments

Comments
 (0)