mirror of https://github.com/openclaw/openclaw.git
Security: block exec approval shell carrier targets (#57871)
* Security: block exec approval shell carrier targets * Tests: tighten exec approval carrier regression assertions
This commit is contained in:
Agustin Rivera
GitHub
parent
9d9cf0d8ff
commit
e65c265e89
|
|
@ -120,38 +120,31 @@ describe("resolveAllowAlwaysPatterns", () => {
|
|||
expect(second.allowlistSatisfied).toBe(true);
|
||||
}
|
||||
|
||||
function expectPositionalArgvCarrierResult(params: {
|
||||
command: string;
|
||||
expectPersisted: boolean;
|
||||
}) {
|
||||
function expectPositionalArgvCarrierRejected(command: string) {
|
||||
const dir = makeTempDir();
|
||||
const touch = makeExecutable(dir, "touch");
|
||||
const env = { PATH: `${dir}${path.delimiter}${process.env.PATH ?? ""}` };
|
||||
const safeBins = resolveSafeBins(undefined);
|
||||
const marker = path.join(dir, "marker");
|
||||
const command = params.command.replaceAll("{marker}", marker);
|
||||
const expandedCommand = command.replaceAll("{marker}", marker);
|
||||
|
||||
const { persisted } = resolvePersistedPatterns({
|
||||
command,
|
||||
command: expandedCommand,
|
||||
dir,
|
||||
env,
|
||||
safeBins,
|
||||
});
|
||||
if (params.expectPersisted) {
|
||||
expect(persisted).toEqual([touch]);
|
||||
} else {
|
||||
expect(persisted).not.toContain(touch);
|
||||
}
|
||||
expect(persisted).toEqual([]);
|
||||
|
||||
const second = evaluateShellAllowlist({
|
||||
command,
|
||||
command: expandedCommand,
|
||||
allowlist: [{ pattern: touch }],
|
||||
safeBins,
|
||||
cwd: dir,
|
||||
env,
|
||||
platform: process.platform,
|
||||
});
|
||||
expect(second.allowlistSatisfied).toBe(params.expectPersisted);
|
||||
expect(second.allowlistSatisfied).toBe(false);
|
||||
}
|
||||
|
||||
it("returns direct executable paths for non-shell segments", () => {
|
||||
|
|
@ -290,63 +283,34 @@ describe("resolveAllowAlwaysPatterns", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("persists carried executables for shell-wrapper positional argv carriers", () => {
|
||||
it("rejects shell-wrapper positional argv carriers", () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
const dir = makeTempDir();
|
||||
const touch = makeExecutable(dir, "touch");
|
||||
const env = { PATH: `${dir}${path.delimiter}${process.env.PATH ?? ""}` };
|
||||
const safeBins = resolveSafeBins(undefined);
|
||||
|
||||
const { persisted } = resolvePersistedPatterns({
|
||||
command: `sh -lc '$0 "$1"' touch ${path.join(dir, "marker")}`,
|
||||
dir,
|
||||
env,
|
||||
safeBins,
|
||||
});
|
||||
expect(persisted).toEqual([touch]);
|
||||
|
||||
const second = evaluateShellAllowlist({
|
||||
command: `sh -lc '$0 "$1"' touch ${path.join(dir, "second-marker")}`,
|
||||
allowlist: [{ pattern: touch }],
|
||||
safeBins,
|
||||
cwd: dir,
|
||||
env,
|
||||
platform: process.platform,
|
||||
});
|
||||
expect(second.allowlistSatisfied).toBe(true);
|
||||
expectPositionalArgvCarrierRejected(`sh -lc '$0 "$1"' touch {marker}`);
|
||||
});
|
||||
|
||||
it("persists carried executables for exec -- positional argv carriers", () => {
|
||||
it("rejects exec positional argv carriers", () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
expectPositionalArgvCarrierResult({
|
||||
command: `sh -lc 'exec -- "$0" "$1"' touch {marker}`,
|
||||
expectPersisted: true,
|
||||
});
|
||||
expectPositionalArgvCarrierRejected(`sh -lc 'exec -- "$0" "$1"' touch {marker}`);
|
||||
});
|
||||
|
||||
it("rejects positional argv carriers when $0 is single-quoted", () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
expectPositionalArgvCarrierResult({
|
||||
command: `sh -lc "'$0' "$1"" touch {marker}`,
|
||||
expectPersisted: false,
|
||||
});
|
||||
expectPositionalArgvCarrierRejected(`sh -lc "'$0' "$1"" touch {marker}`);
|
||||
});
|
||||
|
||||
it("rejects positional argv carriers when exec is separated from $0 by a newline", () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
expectPositionalArgvCarrierResult({
|
||||
command: `sh -lc "exec
|
||||
expectPositionalArgvCarrierRejected(`sh -lc "exec
|
||||
$0 \\"$1\\"" touch {marker}`,
|
||||
expectPersisted: false,
|
||||
});
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects positional argv carriers when inline command contains extra shell operations", () => {
|
||||
|
|
@ -742,7 +706,7 @@ $0 \\"$1\\"" touch {marker}`,
|
|||
return;
|
||||
}
|
||||
const dir = makeTempDir();
|
||||
makeExecutable(dir, "env");
|
||||
const envPath = makeExecutable(dir, "env");
|
||||
const env = makePathEnv(dir);
|
||||
const safeBins = resolveSafeBins(undefined);
|
||||
|
||||
|
|
@ -756,7 +720,7 @@ $0 \\"$1\\"" touch {marker}`,
|
|||
|
||||
const second = evaluateShellAllowlist({
|
||||
command: `sh -lc '$0 "$@"' env BASH_ENV=/tmp/payload.sh bash -lc 'id > /tmp/pwned'`,
|
||||
allowlist: persisted.map((pattern) => ({ pattern })),
|
||||
allowlist: [{ pattern: envPath }],
|
||||
safeBins,
|
||||
cwd: dir,
|
||||
env,
|
||||
|
|
@ -770,7 +734,7 @@ $0 \\"$1\\"" touch {marker}`,
|
|||
return;
|
||||
}
|
||||
const dir = makeTempDir();
|
||||
makeExecutable(dir, "bash");
|
||||
const bashPath = makeExecutable(dir, "bash");
|
||||
const env = makePathEnv(dir);
|
||||
const safeBins = resolveSafeBins(undefined);
|
||||
|
||||
|
|
@ -784,7 +748,35 @@ $0 \\"$1\\"" touch {marker}`,
|
|||
|
||||
const second = evaluateShellAllowlist({
|
||||
command: `sh -lc '$0 "$@"' bash -lc 'id > /tmp/pwned'`,
|
||||
allowlist: persisted.map((pattern) => ({ pattern })),
|
||||
allowlist: [{ pattern: bashPath }],
|
||||
safeBins,
|
||||
cwd: dir,
|
||||
env,
|
||||
platform: process.platform,
|
||||
});
|
||||
expect(second.allowlistSatisfied).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects positional carrier when carried executable is an unknown dispatch carrier", () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
const dir = makeTempDir();
|
||||
const xargsPath = makeExecutable(dir, "xargs");
|
||||
const env = makePathEnv(dir);
|
||||
const safeBins = resolveSafeBins(undefined);
|
||||
|
||||
const { persisted } = resolvePersistedPatterns({
|
||||
command: `sh -lc '$0 "$@"' xargs echo SAFE`,
|
||||
dir,
|
||||
env,
|
||||
safeBins,
|
||||
});
|
||||
expect(persisted).toEqual([]);
|
||||
|
||||
const second = evaluateShellAllowlist({
|
||||
command: `sh -lc '$0 "$@"' xargs sh -lc 'id > /tmp/pwned'`,
|
||||
allowlist: [{ pattern: xargsPath }],
|
||||
safeBins,
|
||||
cwd: dir,
|
||||
env,
|
||||
|
|
|
|||
|
|
@ -1,5 +1,4 @@
|
|||
import path from "node:path";
|
||||
import { isDispatchWrapperExecutable } from "./dispatch-wrapper-resolution.js";
|
||||
import {
|
||||
analyzeShellCommand,
|
||||
isWindowsPlatform,
|
||||
|
|
@ -26,11 +25,9 @@ import { isTrustedSafeBinPath } from "./exec-safe-bin-trust.js";
|
|||
import {
|
||||
extractShellWrapperInlineCommand,
|
||||
isShellWrapperExecutable,
|
||||
normalizeExecutableToken,
|
||||
} from "./exec-wrapper-resolution.js";
|
||||
import { resolveExecWrapperTrustPlan } from "./exec-wrapper-trust-plan.js";
|
||||
import { expandHomePrefix } from "./home-dir.js";
|
||||
import { POSIX_INLINE_COMMAND_FLAGS, resolveInlineCommandMatch } from "./shell-inline-command.js";
|
||||
|
||||
function hasShellLineContinuation(command: string): boolean {
|
||||
return /\\(?:\r\n|\n|\r)/.test(command);
|
||||
|
|
@ -235,18 +232,6 @@ function evaluateSegments(
|
|||
: executableResolution;
|
||||
const executableMatch = matchAllowlist(params.allowlist, candidateResolution);
|
||||
const inlineCommand = extractShellWrapperInlineCommand(allowlistSegment.argv);
|
||||
const shellPositionalArgvCandidatePath = resolveShellWrapperPositionalArgvCandidatePath({
|
||||
segment: allowlistSegment,
|
||||
cwd: params.cwd,
|
||||
env: params.env,
|
||||
});
|
||||
const shellPositionalArgvMatch = shellPositionalArgvCandidatePath
|
||||
? matchAllowlist(params.allowlist, {
|
||||
rawExecutable: shellPositionalArgvCandidatePath,
|
||||
resolvedPath: shellPositionalArgvCandidatePath,
|
||||
executableName: path.basename(shellPositionalArgvCandidatePath),
|
||||
})
|
||||
: null;
|
||||
const shellScriptCandidatePath =
|
||||
inlineCommand === null
|
||||
? resolveShellWrapperScriptCandidatePath({
|
||||
|
|
@ -261,7 +246,7 @@ function evaluateSegments(
|
|||
executableName: path.basename(shellScriptCandidatePath),
|
||||
})
|
||||
: null;
|
||||
const match = executableMatch ?? shellPositionalArgvMatch ?? shellScriptMatch;
|
||||
const match = executableMatch ?? shellScriptMatch;
|
||||
if (match) {
|
||||
matches.push(match);
|
||||
}
|
||||
|
|
@ -428,71 +413,6 @@ function resolveShellWrapperScriptCandidatePath(params: {
|
|||
return path.resolve(base, expanded);
|
||||
}
|
||||
|
||||
function resolveShellWrapperPositionalArgvCandidatePath(params: {
|
||||
segment: ExecCommandSegment;
|
||||
cwd?: string;
|
||||
env?: NodeJS.ProcessEnv;
|
||||
}): string | undefined {
|
||||
if (!isShellWrapperSegment(params.segment)) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const argv = params.segment.argv;
|
||||
if (!Array.isArray(argv) || argv.length < 4) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const wrapper = normalizeExecutableToken(argv[0] ?? "");
|
||||
if (!["ash", "bash", "dash", "fish", "ksh", "sh", "zsh"].includes(wrapper)) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const inlineMatch = resolveInlineCommandMatch(argv, POSIX_INLINE_COMMAND_FLAGS, {
|
||||
allowCombinedC: true,
|
||||
});
|
||||
if (inlineMatch.valueTokenIndex === null || !inlineMatch.command) {
|
||||
return undefined;
|
||||
}
|
||||
if (!isDirectShellPositionalCarrierInvocation(inlineMatch.command)) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const carriedExecutable = argv
|
||||
.slice(inlineMatch.valueTokenIndex + 1)
|
||||
.map((token) => token.trim())
|
||||
.find((token) => token.length > 0);
|
||||
if (!carriedExecutable) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
// Reject wrapper targets carried through `$0 "$@"` because their trailing argv can
|
||||
// widen execution semantics beyond the original approved command.
|
||||
const carriedName = normalizeExecutableToken(carriedExecutable);
|
||||
if (isDispatchWrapperExecutable(carriedName) || isShellWrapperExecutable(carriedName)) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const resolution = resolveCommandResolutionFromArgv([carriedExecutable], params.cwd, params.env);
|
||||
return resolveExecutionTargetCandidatePath(resolution, params.cwd);
|
||||
}
|
||||
|
||||
function isDirectShellPositionalCarrierInvocation(command: string): boolean {
|
||||
const trimmed = command.trim();
|
||||
if (trimmed.length === 0) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Keep carrier matching strict: only allow direct `$0` execution with positional arguments.
|
||||
// This prevents payloads like `echo blocked; $0 "$1"` from satisfying allowlist checks.
|
||||
const shellWhitespace = String.raw`[^\S\r\n]+`;
|
||||
const positionalZero = String.raw`(?:\$(?:0|\{0\})|"\$(?:0|\{0\})")`;
|
||||
const positionalArg = String.raw`(?:\$(?:[@*]|[1-9]|\{[@*1-9]\})|"\$(?:[@*]|[1-9]|\{[@*1-9]\})")`;
|
||||
return new RegExp(
|
||||
`^(?:exec${shellWhitespace}(?:--${shellWhitespace})?)?${positionalZero}(?:${shellWhitespace}${positionalArg})*$`,
|
||||
"u",
|
||||
).test(trimmed);
|
||||
}
|
||||
|
||||
function collectAllowAlwaysPatterns(params: {
|
||||
segment: ExecCommandSegment;
|
||||
cwd?: string;
|
||||
|
|
@ -529,18 +449,6 @@ function collectAllowAlwaysPatterns(params: {
|
|||
params.out.add(candidatePath);
|
||||
return;
|
||||
}
|
||||
const positionalArgvPath = resolveShellWrapperPositionalArgvCandidatePath({
|
||||
segment,
|
||||
cwd: params.cwd,
|
||||
env: params.env,
|
||||
});
|
||||
if (positionalArgvPath) {
|
||||
if (isInterpreterLikeAllowlistPattern(positionalArgvPath)) {
|
||||
return;
|
||||
}
|
||||
params.out.add(positionalArgvPath);
|
||||
return;
|
||||
}
|
||||
const inlineCommand =
|
||||
trustPlan.shellInlineCommand ?? extractShellWrapperInlineCommand(segment.argv);
|
||||
if (!inlineCommand) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue