diff --git a/CHANGELOG.md b/CHANGELOG.md index f54916058f..a8cd6f85be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,7 @@ Docs: https://docs.openclaw.ai ### Fixes - Security/Voice Call: harden `voice-call` telephony TTS override merging by blocking unsafe deep-merge keys (`__proto__`, `prototype`, `constructor`) and add regression coverage for top-level and nested prototype-pollution payloads. -- Security/Windows Daemon: escape and quote Scheduled Task environment assignments when generating `gateway.cmd` (`set "KEY=VALUE"`), preventing command injection from config-provided env vars. This ships in the next npm release. Thanks @tdjackey for reporting. +- Security/Windows Daemon: harden Scheduled Task `gateway.cmd` generation by quoting cmd metacharacter arguments, escaping `%`/`!` expansions, and rejecting CR/LF in arguments, descriptions, and environment assignments (`set "KEY=VALUE"`), preventing command injection in Windows daemon startup scripts. This ships in the next npm release. Thanks @tdjackey for reporting. - Security/Gateway/Canvas: replace shared-IP fallback auth with node-scoped session capability URLs for `/__openclaw__/canvas/*` and `/__openclaw__/a2ui/*`, fail closed when trusted-proxy requests omit forwarded client headers, and add IPv6/proxy-header regression coverage. This ships in the next npm release. Thanks @aether-ai-agent for reporting. - Security/Net: enforce strict dotted-decimal IPv4 literals in SSRF checks and fail closed on unsupported legacy forms (octal/hex/short/packed, for example `0177.0.0.1`, `127.1`, `2130706433`) before DNS lookup. - Security/Discord: enforce trusted-sender guild permission checks for moderation actions (`timeout`, `kick`, `ban`) and ignore untrusted `senderUserId` params to prevent privilege escalation in tool-driven flows. Thanks @aether-ai-agent for reporting. diff --git a/src/daemon/schtasks.install.test.ts b/src/daemon/schtasks.install.test.ts index fb5458dc1f..c7bfb41710 100644 --- a/src/daemon/schtasks.install.test.ts +++ b/src/daemon/schtasks.install.test.ts @@ -29,7 +29,17 @@ describe("installScheduledTask", () => { const { scriptPath } = await installScheduledTask({ env, stdout: new PassThrough(), - programArguments: ["node", "gateway.js", "--verbose"], + programArguments: [ + "node", + "gateway.js", + "--display-name", + "safe&whoami", + "--percent", + "%TEMP%", + "--bang", + "!token!", + ], + workingDirectory: "C:\\temp\\poc&calc", environment: { OC_INJECT: "safe & whoami | calc", OC_CARET: "a^b", @@ -40,6 +50,10 @@ describe("installScheduledTask", () => { }); const script = await fs.readFile(scriptPath, "utf8"); + expect(script).toContain('cd /d "C:\\temp\\poc&calc"'); + expect(script).toContain( + 'node gateway.js --display-name "safe&whoami" --percent "%%TEMP%%" --bang "^!token^!"', + ); expect(script).toContain('set "OC_INJECT=safe & whoami | calc"'); expect(script).toContain('set "OC_CARET=a^^b"'); expect(script).toContain('set "OC_PERCENT=%%TEMP%%"'); @@ -48,6 +62,19 @@ describe("installScheduledTask", () => { expect(script).not.toContain("set OC_INJECT="); const parsed = await readScheduledTaskCommand(env); + expect(parsed).toMatchObject({ + programArguments: [ + "node", + "gateway.js", + "--display-name", + "safe&whoami", + "--percent", + "%TEMP%", + "--bang", + "!token!", + ], + workingDirectory: "C:\\temp\\poc&calc", + }); expect(parsed?.environment).toMatchObject({ OC_INJECT: "safe & whoami | calc", OC_CARET: "a^b", @@ -63,4 +90,43 @@ describe("installScheduledTask", () => { await fs.rm(tmpDir, { recursive: true, force: true }); } }); + + it("rejects line breaks in command arguments, env vars, and descriptions", async () => { + const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-schtasks-install-")); + const env = { + USERPROFILE: tmpDir, + OPENCLAW_PROFILE: "default", + }; + try { + await expect( + installScheduledTask({ + env, + stdout: new PassThrough(), + programArguments: ["node", "gateway.js", "bad\narg"], + environment: {}, + }), + ).rejects.toThrow(/Command argument cannot contain CR or LF/); + + await expect( + installScheduledTask({ + env, + stdout: new PassThrough(), + programArguments: ["node", "gateway.js"], + environment: { BAD: "line1\r\nline2" }, + }), + ).rejects.toThrow(/Environment variable value cannot contain CR or LF/); + + await expect( + installScheduledTask({ + env, + stdout: new PassThrough(), + description: "bad\ndescription", + programArguments: ["node", "gateway.js"], + environment: {}, + }), + ).rejects.toThrow(/Task description cannot contain CR or LF/); + } finally { + await fs.rm(tmpDir, { recursive: true, force: true }); + } + }); }); diff --git a/src/daemon/schtasks.ts b/src/daemon/schtasks.ts index 4085fefefd..ab64c8c22a 100644 --- a/src/daemon/schtasks.ts +++ b/src/daemon/schtasks.ts @@ -36,13 +36,35 @@ export function resolveTaskScriptPath(env: GatewayServiceEnv): string { return path.join(stateDir, scriptName); } -function quoteCmdArg(value: string): string { +function assertNoCmdLineBreak(value: string, field: string): void { + if (/[\r\n]/.test(value)) { + throw new Error(`${field} cannot contain CR or LF in Windows task scripts.`); + } +} + +function quoteSchtasksArg(value: string): string { if (!/[ \t"]/g.test(value)) { return value; } return `"${value.replace(/"/g, '\\"')}"`; } +function quoteCmdScriptArg(value: string): string { + assertNoCmdLineBreak(value, "Command argument"); + if (!value) { + return '""'; + } + const escaped = value.replace(/"/g, '\\"').replace(/%/g, "%%").replace(/!/g, "^!"); + if (!/[ \t"&|<>^()%!]/g.test(value)) { + return escaped; + } + return `"${escaped}"`; +} + +function unescapeCmdScriptArg(value: string): string { + return value.replace(/\^!/g, "!").replace(/%%/g, "%"); +} + function resolveTaskUser(env: GatewayServiceEnv): string | null { const username = env.USERNAME || env.USER || env.LOGNAME; if (!username) { @@ -59,9 +81,11 @@ function resolveTaskUser(env: GatewayServiceEnv): string | null { } function parseCommandLine(value: string): string[] { - // `buildTaskScript` only escapes quotes (`\"`). + // `buildTaskScript` escapes quotes (`\"`) and cmd expansions (`%%`, `^!`). // Keep all other backslashes literal so drive and UNC paths are preserved. - return splitArgsPreservingQuotes(value, { escapeMode: "backslash-quote-only" }); + return splitArgsPreservingQuotes(value, { escapeMode: "backslash-quote-only" }).map( + unescapeCmdScriptArg, + ); } export async function readScheduledTaskCommand( @@ -143,21 +167,25 @@ function buildTaskScript({ environment, }: GatewayServiceRenderArgs): string { const lines: string[] = ["@echo off"]; - if (description?.trim()) { - lines.push(`rem ${description.trim()}`); + const trimmedDescription = description?.trim(); + if (trimmedDescription) { + assertNoCmdLineBreak(trimmedDescription, "Task description"); + lines.push(`rem ${trimmedDescription}`); } if (workingDirectory) { - lines.push(`cd /d ${quoteCmdArg(workingDirectory)}`); + lines.push(`cd /d ${quoteCmdScriptArg(workingDirectory)}`); } if (environment) { for (const [key, value] of Object.entries(environment)) { if (!value) { continue; } + assertNoCmdLineBreak(key, "Environment variable name"); + assertNoCmdLineBreak(value, "Environment variable value"); lines.push(renderCmdSetAssignment(key, value)); } } - const command = programArguments.map(quoteCmdArg).join(" "); + const command = programArguments.map(quoteCmdScriptArg).join(" "); lines.push(command); return `${lines.join("\r\n")}\r\n`; } @@ -192,7 +220,7 @@ export async function installScheduledTask({ await fs.writeFile(scriptPath, script, "utf8"); const taskName = resolveTaskName(env); - const quotedScript = quoteCmdArg(scriptPath); + const quotedScript = quoteSchtasksArg(scriptPath); const baseArgs = [ "/Create", "/F",