GHSA-hf57-cqmx-p4gr OmniRoute ACP Custom-Agent Remote Code Execution (RCE)

• npm omniroute: <= 3.8.50 (no fix yet)

2. Summary `POST /api/acp/agents` registers a custom ACP agent. The endpoint accepts user-controlled `binary` and `versionCommand` values. After saving the custom agent, the same request calls `refreshAgentCache()`, which triggers agent version detection. The version probe eventually runs: ```ts execFileSync(probe.command, probe.args, ...) ``` The only validation is `resolveVersionProbe(binary, versionCommand, true)`, which checks that the first token of `versionCommand` matches the request-provided `binary`. Because `binary` is also attacker-controlled, an attacker can submit: ```json { "binary": "node", "versionCommand": "node -e \"...arbitrary JavaScript...\"" } ``` This executes arbitrary Node.js code inside the server container, and that code can execute OS commands via `child_process.execSync()`. When `requireLogin=false`, `isAuthenticated()` treats anonymous requests as authenticated. At the same time, `/api/acp/` is not included in `LOCAL_ONLY_API_PREFIXES` or `SPAWN_CAPABLE_PREFIXES`, so the endpoint is not blocked by the LOCAL_ONLY policy before reaching the anonymous allow branch. As a result, a remote anonymous attacker can execute commands inside the OmniRoute container with a single HTTP request. 3. Preconditions The unauthenticated exploit is reachable in either of the following scenarios: 1. The target instance has `requireLogin=false`. This is the primary scenario covered by this report and by the reproduction steps below. 2. A fresh instance has no management password configured yet. During this bootstrap window, `/api/settings/require-login` allows unauthenticated setup writes, so an attacker can first set `requireLogin=false` and then call the vulnerable endpoint. If the instance is in the default `requireLogin=true` state and already has a management password, exploitation requires a valid management session or management-scoped API key. In that case, the bug is authenticated RCE rather than the unauthenticated scenario emphasized here. 4. Technical Analysis 4.1 The Endpoint Accepts User-Controlled Command Fields `src/app/api/acp/agents/route.ts:15-24` defines a request schema that accepts `binary`, `versionCommand`, and `spawnArgs`: ```ts const customAgentBodySchema = z.object({ action: z.string().optional(), id: z.string().optional(), name: z.string().optional(), binary: z.string().optional(), versionCommand: z.string().optional(), providerAlias: z.string().optional(), spawnArgs: z.array(z.string()).optional(), protocol: z.enum(["stdio", "http"]).optional(), }); ``` The `POST` handler at `src/app/api/acp/agents/route.ts:58-61` only calls `isAuthenticated()`: ```ts export async function POST(request: Request) { if (!(await isAuthenticated(request))) { return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); } ``` The handler then stores `binary` and `versionCommand` in the custom agent definition without an executable allowlist: ```ts const newAgent: CustomAgentDef = { id: id.toLowerCase().replace(/[^a-z0-9-]/g, "-"), name, binary, versionCommand, providerAlias: providerAlias || id, spawnArgs: spawnArgs || [], protocol: protocol || "stdio", }; ``` This logic is in `src/app/api/acp/agents/route.ts:92-100`. 4.2 The Only Guard Is a Self-Consistency Check The only command validation in the route is at `src/app/api/acp/agents/route.ts:102-107`: ```ts if (!resolveVersionProbe(newAgent.binary, newAgent.versionCommand, true)) { return NextResponse.json( { error: "Invalid versionCommand: use the configured binary with plain arguments only" }, { status: 400 } ); } ``` The core logic of `resolveVersionProbe()` is in `src/lib/acp/registry.ts:261-288`: ```ts export function resolveVersionProbe( binary: string, versionCommand: string, requireBinaryMatch = false ): { command: string; args: string[] } | null { const tokens = tokenizeVersionCommand(versionCommand); if (!tokens) { return null; } const [command, ...args] = tokens; if (!command) { return null; } if (requireBinaryMatch) { const normalizedCommand = normalizeCommandToken(command); const allowed = new Set([ normalizeCommandToken(binary), normalizeCommandToken(path.basename(binary)), ]); if (!allowed.has(normalizedCommand)) { return null; } } return { command, args }; } ``` This check only requires the first token of `versionCommand` to equal `binary` or `path.basename(binary)`. Since `binary` is also attacker-controlled, `binary="node"` and `versionCommand="node -e \"...\""` pass validation. `tokenizeVersionCommand()` only blocks a small set of shell metacharacters (`src/lib/acp/registry.ts:183-254`): ```ts const DISALLOWED_VERSION_COMMAND_CHARS = /[;&|<>`$\r\n]/; ``` This does not prevent `node -e` code execution, because characters needed for the payload, such as `(`, `)`, `'`, `.`, `/`, `,`, and spaces, are allowed. 4.3 The Same Request Immediately Triggers Command Execution After saving the custom agent, the route calls `refreshAgentCache()` at `src/app/api/acp/agents/route.ts:121-127`: ```ts const updated = [...current, newAgent]; await updateSettings({ customAgents: updated }); setCustomAgents(updated); const agents = refreshAgentCache(); return NextResponse.json({ agents, added: newAgent }); ``` `refreshAgentCache()` is defined at `src/lib/acp/registry.ts:366-369`: ```ts export function refreshAgentCache(): CliAgentInfo[] { _cachedAgents = null; return detectInstalledAgents(); } ``` `detectInstalledAgents()` merges built-in and custom agents and calls `detectAgent()` for each one (`src/lib/acp/registry.ts:342-360`): ```ts const allDefs = [ ...AGENT_DEFINITIONS.map((d) => ({ ...d, _custom: false })), ..._customAgentDefs.map((d) => ({ ...d, _custom: true })), ]; _cachedAgents = allDefs.map((def) => { const { _custom, ...rest } = def; return detectAgent(rest, _custom); }); ``` The command execution sink is at `src/lib/acp/registry.ts:307-325`: ```ts const probe = resolveVersionProbe(def.binary, def.versionCommand, isCustom); if (!probe) { return { ...def, version, installed, isCustom }; } const output = execFileSync(probe.command, probe.args, { timeout: 5000, encoding: "utf-8", stdio: ["pipe", "pipe", "pipe"], ...(shouldUseShellForVersionProbe(probe.command) ? { shell: true } : {}), }).trim(); ``` On Linux containers, `shouldUseShellForVersionProbe()` returns `false` for non-Windows platforms (`src/lib/acp/registry.ts:290-301`): ```ts export function shouldUseShellForVersionProbe( command: string, platform = process.platform ): boolean { if (platform !== "win32") return false; ... } ``` Therefore the effective execution is `execFileSync("node", ["-e", "..."])`. No shell metacharacters are required. 4.4 Why This Is Unauthenticated `isAuthenticated()` is defined at `src/shared/utils/apiAuth.ts:285-302`: ```ts export async function isAuthenticated(request: Request): Promise { if (!(await isAuthRequired(request))) { return true; } ... } ``` `isAuthRequired()` returns `false` when `requireLogin=false` (`src/shared/utils/apiAuth.ts:317-323`): ```ts const settings = await getSettings(); if (settings.requireLogin === false) return false; ``` The centralized management policy also has the same anonymous allow branch at `src/server/authz/policies/management.ts:223-226`: ```ts if (!isAlwaysProtectedPath(path) && !(await isAuthRequired(ctx.request))) { return allow({ kind: "anonymous", id: "anonymous", label: "auth-disabled" }); } ``` Routes that can start local subprocesses should be blocked by the LOCAL_ONLY policy first. `src/server/authz/routeGuard.ts:29-45` lists LOCAL_ONLY prefixes such as `/api/mcp/`, `/api/cli-tools/runtime/`, `/api/services/`, `/api/tools/agent-bridge/`, and `/api/plugins/`, but it does not include `/api/acp/`: ```ts export const LOCAL_ONLY_API_PREFIXES: ReadonlyArray = [ "/api/mcp/", "/api/cli-tools/runtim…