diff --git a/docs/apktool-resource-analysis.md b/docs/apktool-resource-analysis.md index 317437ec1..f29d7948c 100644 --- a/docs/apktool-resource-analysis.md +++ b/docs/apktool-resource-analysis.md @@ -22,6 +22,14 @@ reports the resolved launcher, its selection source, and its version. A missing launcher reports `capability_unavailable` with the `REA_APKTOOL_COMMAND` remediation rather than attempting an installation. +On Windows, a `.bat` or `.cmd` launcher (such as Chocolatey's `apktool.bat`) +runs through an explicit `cmd.exe /d /s /c` call. That call has no AutoRun, +no delayed expansion, and quotes every argument. A launcher path that +contains `"`, `%`, `^`, `!`, `&`, `|`, `<` or `>` is reported as +`not_configured` instead of being passed to `cmd.exe`. If REA's temporary +decode paths contain those characters, decoding through a batch launcher +reports `capability_unavailable`. + ## Answer an analyst question ```sh diff --git a/src/apktool/ApktoolConfiguration.test.ts b/src/apktool/ApktoolConfiguration.test.ts index 3657f10ef..d2d84874c 100644 --- a/src/apktool/ApktoolConfiguration.test.ts +++ b/src/apktool/ApktoolConfiguration.test.ts @@ -2,6 +2,10 @@ import { describe, expect, it } from "vitest"; import { APKTOOL_COMMAND_ENV, + apktoolConfigurationAvailability, + ApktoolConfigurationFailure, + apktoolInvocation, + inspectApktoolClient, parseApktoolVersionOutput, resolveApktoolCommand, } from "./ApktoolConfiguration.js"; @@ -43,3 +47,75 @@ describe("parseApktoolVersionOutput", () => { expect(parseApktoolVersionOutput(" \n")).toEqual({ version: null }); }); }); + +describe("apktoolInvocation", () => { + const windows = { SystemRoot: "C:\\Windows" }; + + it("runs a Windows batch launcher through an explicit cmd.exe call", () => { + expect( + apktoolInvocation( + { + command: "C:\\ProgramData\\chocolatey\\bin\\apktool.BAT", + commandSource: "environment", + }, + ["d", "-o", "C:\\Users\\Analyst\\AppData\\Local\\Temp\\rea out"], + windows, + "win32", + ), + ).toEqual({ + command: "C:\\Windows\\System32\\cmd.exe", + arguments: [ + "/d", + "/e:on", + "/v:off", + "/s", + "/c", + '""C:\\ProgramData\\chocolatey\\bin\\apktool.BAT" "d" "-o" "C:\\Users\\Analyst\\AppData\\Local\\Temp\\rea out""', + ], + windowsVerbatimArguments: true, + }); + }); + + it("runs native executables and POSIX scripts directly", () => { + for (const [command, platform] of [ + ["C:\\tools\\apktool.exe", "win32"], + ["/opt/apktool/apktool.cmd", "linux"], + ] as const) + expect( + apktoolInvocation( + { command, commandSource: "environment" }, + ["--version"], + windows, + platform, + ), + ).toEqual({ + command, + arguments: ["--version"], + windowsVerbatimArguments: false, + }); + }); + + it("reports a batch launcher that cmd.exe would reinterpret before running it", async () => { + const failure = await inspectApktoolClient( + { + command: "C:\\tools & more\\apktool.cmd", + commandSource: "environment", + }, + undefined, + windows, + "win32", + ).catch((cause: unknown) => cause); + + expect(failure).toBeInstanceOf(ApktoolConfigurationFailure); + if (!(failure instanceof ApktoolConfigurationFailure)) return; + expect(failure.code).toBe("launcher_unsupported"); + expect(apktoolConfigurationAvailability(failure)).toMatchObject({ + status: "unavailable", + code: "not_configured", + diagnostics: { + selected_command: "C:\\tools & more\\apktool.cmd", + remediation: expect.stringContaining(APKTOOL_COMMAND_ENV), + }, + }); + }); +}); diff --git a/src/apktool/ApktoolConfiguration.ts b/src/apktool/ApktoolConfiguration.ts index 9f0c3a9ef..12532b104 100644 --- a/src/apktool/ApktoolConfiguration.ts +++ b/src/apktool/ApktoolConfiguration.ts @@ -8,6 +8,11 @@ import { import type { ProviderAvailability } from "../application/AnalysisProvider.js"; import type { ProviderRejectionCode } from "../contracts/providerSelection.js"; import type { JsonValue } from "../domain/jsonValue.js"; +import { + isWindowsBatchFile, + windowsBatchCommand, + WindowsBatchCommandError, +} from "../process/WindowsBatchCommand.js"; /** Environment key selecting the caller's apktool launcher. */ export const APKTOOL_COMMAND_ENV = "REA_APKTOOL_COMMAND"; @@ -46,6 +51,7 @@ export const resolveApktoolCommand = ( export type ApktoolConfigurationFailureCode = | "not_absolute" | "command_missing" + | "launcher_unsupported" | "version_unresolved"; export class ApktoolConfigurationFailure extends Error { @@ -63,6 +69,40 @@ export interface ApktoolClientIdentity { readonly version: string | null; } +/** Process creation for one apktool run on the selected host. */ +export interface ApktoolInvocation { + readonly command: string; + readonly arguments: readonly string[]; + readonly windowsVerbatimArguments: boolean; +} + +/** + * Run the selected launcher directly, or through cmd.exe when Windows can + * only execute it there (the Chocolatey `apktool.bat` and `java -jar` + * wrapper scripts). Throws WindowsBatchCommandError when a token cannot be + * passed to cmd.exe safely. + */ +export const apktoolInvocation = ( + selection: ApktoolCommandSelection, + arguments_: readonly string[], + environment: Readonly, + platform: NodeJS.Platform = process.platform, +): ApktoolInvocation => + platform === "win32" && isWindowsBatchFile(selection.command) + ? { + ...windowsBatchCommand({ + environment, + batchPath: selection.command, + arguments: arguments_, + }), + windowsVerbatimArguments: true, + } + : { + command: selection.command, + arguments: arguments_, + windowsVerbatimArguments: false, + }; + /** Read the version from `apktool --version` output (first non-empty line). */ export const parseApktoolVersionOutput = ( stdout: string, @@ -79,15 +119,43 @@ export const inspectApktoolClient = async ( selection: ApktoolCommandSelection, signal?: AbortSignal, environment: Readonly> = process.env, + platform: NodeJS.Platform = process.platform, ): Promise => { + const hostEnvironment = { ...process.env, ...environment }; + let invocation: ApktoolInvocation; try { - const { stdout } = await execFileOutput(selection.command, ["--version"], { - timeout: VERSION_TIMEOUT_MS, - stopSignal: "SIGTERM", - maxBuffer: VERSION_MAX_OUTPUT_BYTES, - env: { ...process.env, ...environment }, - ...(signal === undefined ? {} : { signal }), - }); + invocation = apktoolInvocation( + selection, + ["--version"], + hostEnvironment, + platform, + ); + } catch (cause) { + if (!(cause instanceof WindowsBatchCommandError)) throw cause; + throw new ApktoolConfigurationFailure( + "launcher_unsupported", + `The selected apktool launcher cannot be run through cmd.exe: ${cause.message}`, + { + selected_command: selection.command, + command_source: selection.commandSource, + remediation: `Set ${APKTOOL_COMMAND_ENV} to a launcher path without cmd.exe metacharacters (" % ^ ! & | < >), or to a native apktool executable`, + }, + { cause }, + ); + } + try { + const { stdout } = await execFileOutput( + invocation.command, + invocation.arguments, + { + windowsVerbatimArguments: invocation.windowsVerbatimArguments, + timeout: VERSION_TIMEOUT_MS, + stopSignal: "SIGTERM", + maxBuffer: VERSION_MAX_OUTPUT_BYTES, + env: hostEnvironment, + ...(signal === undefined ? {} : { signal }), + }, + ); return parseApktoolVersionOutput(stdout); } catch (cause) { const failure = execFileOutputFailure(cause); @@ -127,7 +195,7 @@ const rejectionCodeOf = ( ): ProviderRejectionCode => code === "command_missing" ? "executable_missing" - : code === "not_absolute" + : code === "not_absolute" || code === "launcher_unsupported" ? "not_configured" : "version_unresolved"; diff --git a/src/apktool/ApktoolProvider.ts b/src/apktool/ApktoolProvider.ts index d898b785a..c9a9c2804 100644 --- a/src/apktool/ApktoolProvider.ts +++ b/src/apktool/ApktoolProvider.ts @@ -48,9 +48,11 @@ import { } from "../process/OwnedCommand.js"; import type { ProviderProcessSupervisor } from "../process/ProviderProcess.js"; import { snapshotEnvironment } from "../process/snapshotEnvironment.js"; +import { WindowsBatchCommandError } from "../process/WindowsBatchCommand.js"; import { ApktoolConfigurationFailure, apktoolConfigurationAvailability, + apktoolInvocation, inspectApktoolClient, resolveApktoolCommand, type ApktoolCommandSelection, @@ -136,7 +138,7 @@ interface DecodeLimits { const runApktool = async ( operation: string, - command: string, + selection: ApktoolCommandSelection, arguments_: readonly string[], limits: DecodeLimits, signal: AbortSignal | undefined, @@ -145,13 +147,29 @@ const runApktool = async ( ): Promise> => { if (signal?.aborted === true) return err(new AnalysisCancelledError(operation)); + const hostEnvironment = { ...process.env, ...environment }; + let invocation: ReturnType; + try { + invocation = apktoolInvocation(selection, arguments_, hostEnvironment); + } catch (cause) { + if (!(cause instanceof WindowsBatchCommandError)) throw cause; + return err( + new AnalysisCapabilityUnavailableError( + APKTOOL_PROVIDER_IDENTITY.id, + operation, + `The apktool batch launcher cannot receive REA's decode paths through cmd.exe: ${cause.message}`, + { cause }, + ), + ); + } try { const output = await runOwnedCommand( { - command, - arguments: arguments_, + command: invocation.command, + arguments: invocation.arguments, + windowsVerbatimArguments: invocation.windowsVerbatimArguments, runId: randomUUID(), - hostEnvironment: { ...process.env, ...environment }, + hostEnvironment, }, { timeoutMs: limits.timeoutMs, @@ -219,7 +237,7 @@ const runApktool = async ( return err( new AnalysisUnsupportedTargetError( operation, - command, + selection.command, output?.stderr.text.trim().slice(0, STDERR_EXCERPT_BYTES) || `apktool exited with code ${String(output?.exitCode ?? "unknown")} for ${operation}`, { @@ -586,7 +604,7 @@ export class ApktoolProvider { await mkdir(framework); const decoded = await runApktool( operation, - selection.command, + selection, [ "d", "--no-src", diff --git a/src/ghidra/GhidraLauncher.ts b/src/ghidra/GhidraLauncher.ts index 9488bee2e..a27b16702 100644 --- a/src/ghidra/GhidraLauncher.ts +++ b/src/ghidra/GhidraLauncher.ts @@ -11,6 +11,11 @@ import { basename, dirname, join, posix, win32 } from "node:path"; import writeFileAtomic from "write-file-atomic"; import { windowsPrivateRuntime } from "../windows/WindowsPrivateRuntime.js"; +import { + quoteWindowsBatchToken, + windowsBatchCommand, + WindowsBatchCommandError, +} from "../process/WindowsBatchCommand.js"; import { AnalysisCancelledError } from "../domain/analysisErrorCore.js"; import { err, ok, type Result } from "../domain/result.js"; @@ -322,50 +327,34 @@ export const ghidraHeadlessCommand = (options: { command: options.analyzeHeadlessPath, arguments: [...options.arguments], }; - const environment = snapshotEnvironment( - options.environment, - options.platform, + const command = ghidraBatchStep(() => + windowsBatchCommand({ + environment: options.environment, + batchPath: options.analyzeHeadlessPath, + arguments: options.arguments, + ...(options.comSpec === undefined ? {} : { comSpec: options.comSpec }), + }), ); - const systemRoot = environment.SYSTEMROOT; - const comSpec = - options.comSpec ?? - environment.COMSPEC ?? - (systemRoot === undefined - ? "C:\\Windows\\System32\\cmd.exe" - : win32.join(systemRoot, "System32", "cmd.exe")); - if ( - !win32.isAbsolute(comSpec) || - win32.basename(comSpec).toLowerCase() !== "cmd.exe" - ) - throw new GhidraLaunchError( - "Windows ComSpec must be an absolute cmd.exe path", - ); - const tokens = [options.analyzeHeadlessPath, ...options.arguments].map( - quoteWindowsBatchToken, - ); - const invocation = `"${tokens.join(" ")}"`; - if (invocation.length > 30_000) + if ((command.arguments.at(-1)?.length ?? 0) > 30_000) throw new GhidraLaunchError( "Windows Ghidra command exceeds the P0 length limit", ); - return { - command: comSpec, - arguments: ["/d", "/e:on", "/v:off", "/s", "/c", invocation], - }; + return command; }; -const quoteWindowsBatchToken = (value: string): string => { - if ( - value.length === 0 || - value.includes("\0") || - value.includes("\r") || - value.includes("\n") || - /["%^!&|<>]/u.test(value) - ) +/** Keep Ghidra's launch error type and P0 wording for shared batch checks. */ +const ghidraBatchStep = (step: () => T): T => { + try { + return step(); + } catch (cause: unknown) { + if (!(cause instanceof WindowsBatchCommandError)) throw cause; throw new GhidraLaunchError( - "Windows Ghidra P0 paths cannot contain command-interpreter metacharacters", + cause.reason === "metacharacter" + ? "Windows Ghidra P0 paths cannot contain command-interpreter metacharacters" + : cause.message, + { cause }, ); - return `"${value}"`; + } }; const cleanupStartedProcess = ( @@ -578,7 +567,7 @@ export const ghidraHeadlessJavaOptions = ( if (platform === "win32") return { JDK_JAVA_OPTIONS: [...options, "-XX:-UsePerfData"] - .map(quoteWindowsBatchToken) + .map((option) => ghidraBatchStep(() => quoteWindowsBatchToken(option))) .join(" "), GHIDRA_HEADLESS_JAVA_OPTIONS: "", }; diff --git a/src/process/WindowsBatchCommand.ts b/src/process/WindowsBatchCommand.ts new file mode 100644 index 000000000..52467dde0 --- /dev/null +++ b/src/process/WindowsBatchCommand.ts @@ -0,0 +1,81 @@ +import { win32 } from "node:path"; + +import { snapshotEnvironment } from "./snapshotEnvironment.js"; + +/** Why a batch launcher cannot be passed safely to cmd.exe. */ +export type WindowsBatchCommandFailure = "invalid_comspec" | "metacharacter"; + +export class WindowsBatchCommandError extends Error { + constructor( + readonly reason: WindowsBatchCommandFailure, + message: string, + ) { + super(message); + this.name = "WindowsBatchCommandError"; + } +} + +/** Exact executable and argv passed to shell-free process creation. */ +export interface WindowsBatchCommand { + readonly command: string; + /** Pass verbatim: cmd.exe parses this line itself, not by CRT argv rules. */ + readonly arguments: readonly string[]; +} + +/** + * Windows runs `.bat` and `.cmd` files only through cmd.exe, so Node rejects + * them without a shell (CVE-2024-27980). + */ +export const isWindowsBatchFile = (command: string): boolean => + /\.(?:bat|cmd)$/iu.test(command); + +/** + * Quote one token for cmd.exe. Tokens that cmd.exe could expand, split or + * redirect are rejected instead of escaped. + */ +export const quoteWindowsBatchToken = (value: string): string => { + if ( + value.length === 0 || + value.includes("\0") || + value.includes("\r") || + value.includes("\n") || + /["%^!&|<>]/u.test(value) + ) + throw new WindowsBatchCommandError( + "metacharacter", + "Windows batch arguments cannot contain command-interpreter metacharacters", + ); + return `"${value}"`; +}; + +/** Run a batch file through an explicit, non-AutoRun cmd.exe invocation. */ +export const windowsBatchCommand = (options: { + readonly environment: Readonly; + readonly batchPath: string; + readonly arguments: readonly string[]; + readonly comSpec?: string; +}): WindowsBatchCommand => { + const environment = snapshotEnvironment(options.environment, "win32"); + const systemRoot = environment.SYSTEMROOT; + const comSpec = + options.comSpec ?? + environment.COMSPEC ?? + (systemRoot === undefined + ? "C:\\Windows\\System32\\cmd.exe" + : win32.join(systemRoot, "System32", "cmd.exe")); + if ( + !win32.isAbsolute(comSpec) || + win32.basename(comSpec).toLowerCase() !== "cmd.exe" + ) + throw new WindowsBatchCommandError( + "invalid_comspec", + "Windows ComSpec must be an absolute cmd.exe path", + ); + const tokens = [options.batchPath, ...options.arguments].map( + quoteWindowsBatchToken, + ); + return { + command: comSpec, + arguments: ["/d", "/e:on", "/v:off", "/s", "/c", `"${tokens.join(" ")}"`], + }; +};