mirror of
https://github.com/morluto/rea.git
synced 2026-10-11 21:39:55 +00:00
fix(apktool): run Windows batch launchers through cmd.exe (#1777)
On Windows, Node refuses to spawn .bat and .cmd files without a shell (CVE-2024-27980), so REA_APKTOOL_COMMAND pointing at Chocolatey's apktool.bat or a java -jar wrapper failed its version probe with EINVAL, reported as "did not report a usable version". Move Ghidra's conservative cmd.exe invocation (no AutoRun, no delayed expansion, every token quoted, interpreter metacharacters rejected) into src/process/WindowsBatchCommand.ts. Ghidra keeps its error type, wording and length limit. apktool now uses it for both the version probe and the decode run when the launcher is a batch file on Windows. A launcher that cmd.exe would reinterpret reports launcher_unsupported (not_configured) with remediation instead of version_unresolved. Fixes #1774 Co-authored-by: Claude Opus 5.5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
914ba833de
commit
ae95e68ba5
@@ -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
|
||||
|
||||
@@ -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),
|
||||
},
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<NodeJS.ProcessEnv>,
|
||||
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<Record<string, string | undefined>> = process.env,
|
||||
platform: NodeJS.Platform = process.platform,
|
||||
): Promise<ApktoolClientIdentity> => {
|
||||
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";
|
||||
|
||||
|
||||
@@ -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<Result<{ stdout: string; stderr: string }, AnalysisError>> => {
|
||||
if (signal?.aborted === true)
|
||||
return err(new AnalysisCancelledError(operation));
|
||||
const hostEnvironment = { ...process.env, ...environment };
|
||||
let invocation: ReturnType<typeof apktoolInvocation>;
|
||||
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",
|
||||
|
||||
@@ -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 = <T>(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: "",
|
||||
};
|
||||
|
||||
@@ -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<NodeJS.ProcessEnv>;
|
||||
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(" ")}"`],
|
||||
};
|
||||
};
|
||||
Reference in New Issue
Block a user