From b62931f8335f58bfe344883a4f149e9eef73d835 Mon Sep 17 00:00:00 2001 From: N0zoM1z0 <161784452+N0zoM1z0@users.noreply.github.com> Date: Wed, 7 Oct 2026 03:51:59 +0800 Subject: [PATCH] fix(windows): accept ordinary drive path separators (#791) --- native/windows/README.md | 5 ++ native/windows/src/authority.hpp | 10 ++++ native/windows/src/filesystem.cc | 1 + native/windows/src/process.cc | 5 +- scripts/verify-packaged-ghidra-windows.mjs | 14 ++++-- scripts/verify-windows-native.mjs | 54 ++++++++++++++++++++-- src/ghidra/GhidraLauncher.ts | 11 ++++- src/windows/WindowsOwnedProcess.ts | 4 +- 8 files changed, 94 insertions(+), 10 deletions(-) diff --git a/native/windows/README.md b/native/windows/README.md index f98d974ca..d385dd48b 100644 --- a/native/windows/README.md +++ b/native/windows/README.md @@ -66,6 +66,11 @@ Windows failures retain their constraint, requested coordinate, Win32 code, and system message. Unsupported filesystems and reparse paths are distinct from OS access denial and missing packaged controls. +Ordinary drive-absolute paths accept backslashes, forward slashes, or mixed +separators. File identity retains the original requested spelling. Separator +translation does not resolve dot components, repeated separators, or extend +the supported device/UNC namespaces; existing admission checks still apply. + The bearer-token descriptor remains under its immutable private lease until `runtime_close` on Windows; POSIX removes the descriptor after the bridge reads it. Job ownership guarantees process termination on owner death, not deletion diff --git a/native/windows/src/authority.hpp b/native/windows/src/authority.hpp index 5698b652c..80f52aa5e 100644 --- a/native/windows/src/authority.hpp +++ b/native/windows/src/authority.hpp @@ -19,6 +19,16 @@ namespace rea { +// Translate ordinary DOS drive separators without resolving components or +// changing extended, device, UNC, relative, or drive-relative namespaces. +inline std::wstring ordinaryDriveSeparators(std::wstring path) { + if (path.size() >= 3 && path[1] == L':' && + ((path[0] >= L'A' && path[0] <= L'Z') || (path[0] >= L'a' && path[0] <= L'z')) && + (path[2] == L'\\' || path[2] == L'/')) + std::replace(path.begin(), path.end(), L'/', L'\\'); + return path; +} + struct Failure : std::runtime_error { DWORD win32; std::wstring path; diff --git a/native/windows/src/filesystem.cc b/native/windows/src/filesystem.cc index c1a2288de..d50fc1e29 100644 --- a/native/windows/src/filesystem.cc +++ b/native/windows/src/filesystem.cc @@ -14,6 +14,7 @@ static std::wstring hex(const unsigned char* bytes, size_t count) { } static std::wstring localPath(std::wstring path) { + path = ordinaryDriveSeparators(std::move(path)); if (path.rfind(L"\\\\?\\", 0) == 0) path.erase(0, 4); require(path.size() >= 3 && path[1] == L':' && path[2] == L'\\' && ((path[0] >= L'A' && path[0] <= L'Z') || (path[0] >= L'a' && path[0] <= L'z')), diff --git a/native/windows/src/process.cc b/native/windows/src/process.cc index 3e8028951..fc9dc65a8 100644 --- a/native/windows/src/process.cc +++ b/native/windows/src/process.cc @@ -44,7 +44,8 @@ static void pipe(Handle& read, Handle& write) { std::unique_ptr spawnProcess(const std::wstring& command, const std::wstring& commandLine, const std::wstring& cwd, const std::vector& environment) { - require(command.size() >= 3 && command[1] == L':' && command[2] == L'\\', + const auto executable = ordinaryDriveSeparators(command); + require(executable.size() >= 3 && executable[1] == L':' && executable[2] == L'\\', "Owned Windows process requires an absolute executable path", command, ERROR_INVALID_PARAMETER); auto result = std::make_unique(); result->job = createJob(); @@ -102,7 +103,7 @@ std::unique_ptr spawnProcess(const std::wstring& command, const std::ws if (uniqueEnvironment.empty()) variables.push_back(L'\0'); std::vector line(commandLine.begin(), commandLine.end()); line.push_back(L'\0'); PROCESS_INFORMATION process{}; - require(CreateProcessW(command.c_str(), line.data(), nullptr, nullptr, TRUE, + require(CreateProcessW(executable.c_str(), line.data(), nullptr, nullptr, TRUE, CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT | CREATE_SUSPENDED | CREATE_NO_WINDOW, variables.data(), cwd.empty() ? nullptr : cwd.c_str(), &startup.StartupInfo, &process), "Create process atomically inside owned Job Object failed", command); diff --git a/scripts/verify-packaged-ghidra-windows.mjs b/scripts/verify-packaged-ghidra-windows.mjs index 3df61d288..deee45c4d 100644 --- a/scripts/verify-packaged-ghidra-windows.mjs +++ b/scripts/verify-packaged-ghidra-windows.mjs @@ -109,13 +109,21 @@ const digest = (bytes) => createHash("sha256").update(bytes).digest("hex"); const runtimeParent = join(workspace, "runtime with spaces"); await mkdir(runtimeParent); const environment = { - ...process.env, + ...Object.fromEntries( + Object.entries(process.env).filter( + ([name]) => name.toLowerCase() !== "comspec", + ), + ), + ComSpec: ( + process.env.ComSpec ?? + join(process.env.SystemRoot ?? "C:\\Windows", "System32", "cmd.exe") + ).replaceAll("\\", "/"), REA_ANALYSIS_PROVIDER: "ghidra", REA_LOG_LEVEL: "error", GHIDRA_HEADLESS_MAXMEM: "512m", NODE_OPTIONS: "--max-old-space-size=512", - TEMP: runtimeParent, - TMP: runtimeParent, + TEMP: runtimeParent.replaceAll("\\", "/"), + TMP: runtimeParent.replaceAll("\\", "/"), HOME: workspace, USERPROFILE: workspace, APPDATA: join(workspace, "appdata"), diff --git a/scripts/verify-windows-native.mjs b/scripts/verify-windows-native.mjs index 68c2ce60b..eb079dedb 100644 --- a/scripts/verify-windows-native.mjs +++ b/scripts/verify-windows-native.mjs @@ -79,13 +79,43 @@ try { await mkdir(sourceDirectory); const source = join(sourceDirectory, "target.bin"); await writeFile(source, Buffer.alloc(1024 * 1024, 0x37)); + const sourceIdentity = native.call("open", [source]); + try { + for (const requested of [ + source.replaceAll("\\", "/"), + source.replace(/\\/u, "/"), + ]) { + const admitted = native.call("open", [requested]); + try { + assert.equal(admitted.requestedPath, requested); + assert.equal(admitted.finalPath, sourceIdentity.finalPath); + assert.equal(admitted.fileId, sourceIdentity.fileId); + assert.equal(native.call("read", [admitted.handle, 0, 1])[0], 0x37); + } finally { + native.call("close", [admitted.handle]); + } + } + } finally { + native.call("close", [sourceIdentity.handle]); + } + for (const rejected of [ + source.replace(/^([A-Za-z]):\\/u, "$1:"), + `${sourceDirectory}/../source/target.bin`, + `${sourceDirectory}//target.bin`, + `${source}:stream`, + `\\\\?\\${source.replaceAll("\\", "/")}`, + "//localhost/share/target.bin", + ]) + assert.throws(() => native.call("open", [rejected])); + report.controls.ordinaryDriveSeparatorsAndRequestedIdentity = true; const snapshotOwner = WindowsPrivateRuntime.create( - workspace, + workspace.replace(/\\/u, "/"), "single-flight-", ); try { const rejectedController = new AbortController(); - const accepted = snapshotOwner.snapshot(source, "accepted.bin"); + const requestedSource = source.replaceAll("\\", "/"); + const accepted = snapshotOwner.snapshot(requestedSource, "accepted.bin"); const rejected = snapshotOwner.snapshot( source, "rejected.bin", @@ -94,6 +124,7 @@ try { rejectedController.abort(); await assert.rejects(rejected, /Runtime snapshot is still pending/u); const completed = await accepted; + assert.equal(completed.source.requestedPath, requestedSource); assert.equal( completed.sha256, createHash("sha256") @@ -147,7 +178,7 @@ try { ); assert.throws(() => native.call("open", [source + ":stream"])); const runtime = native.call("runtime_create", [ - workspace, + workspace.replaceAll("\\", "/"), "private root with spaces-", ]); const sentinel = join(sourceDirectory, "sentinel"); @@ -316,6 +347,23 @@ setInterval(()=>{},1000);\n`, assert.match(settled.output, /observed/u); native.call("process_close", [exited.handle]); report.processes.normalExitAndOutput = true; + const forwardCommand = process.execPath.replaceAll("\\", "/"); + const forwardProcess = native.call("process_spawn", [ + forwardCommand, + [forwardCommand, "-e", "console.log('forward executable')"] + .map(quote) + .join(" "), + workspace, + environment, + ]); + try { + const completed = await wait(forwardProcess); + assert.equal(completed.exitCode, 0); + assert.match(completed.output, /forward executable/u); + report.processes.ordinaryDriveSeparators = true; + } finally { + native.call("process_close", [forwardProcess.handle]); + } const boundaryFixture = process.argv[3]; if (boundaryFixture !== undefined) { const before = JSON.parse( diff --git a/src/ghidra/GhidraLauncher.ts b/src/ghidra/GhidraLauncher.ts index bcf105af0..3dfe6567c 100644 --- a/src/ghidra/GhidraLauncher.ts +++ b/src/ghidra/GhidraLauncher.ts @@ -141,7 +141,12 @@ export class GhidraHeadlessLauncher implements GhidraLauncher { expectedCommand: null, windowsVerbatimArguments: platform === "win32", platform, - env: ghidraLaunchEnvironment(paths, this.options.javaHome, platform), + env: ghidraLaunchEnvironment( + paths, + this.options.javaHome, + platform, + command.command, + ), }); await writeGhidraRuntimeFile( paths.ownershipPath, @@ -382,6 +387,7 @@ const ghidraLaunchEnvironment = ( paths: ReturnType, javaHome: string | undefined, platform: NodeJS.Platform, + executable: string, ): NodeJS.ProcessEnv => { return { ...ghidraJavaEnvironment(javaHome, process.env, platform), @@ -390,6 +396,9 @@ const ghidraLaunchEnvironment = ( TMPDIR: paths.tempRoot, ...(platform === "win32" ? { + // Batch FOR /F launches another command interpreter through ComSpec. + // Use the same executable with the spelling consumed by cmd.exe. + ComSpec: executable.replaceAll("/", "\\"), USERPROFILE: paths.homeRoot, APPDATA: paths.configRoot, LOCALAPPDATA: paths.cacheRoot, diff --git a/src/windows/WindowsOwnedProcess.ts b/src/windows/WindowsOwnedProcess.ts index a7d65ada9..fa1abed05 100644 --- a/src/windows/WindowsOwnedProcess.ts +++ b/src/windows/WindowsOwnedProcess.ts @@ -62,7 +62,9 @@ export class WindowsOwnedProcess extends EventEmitter { .filter((entry): entry is [string, string] => entry[1] !== undefined) .map(([key, value]) => `${key}=${value}`); const line = [ - quoteWindowsProcessArgument(command), + // cmd.exe parses its own command line rather than the CRT argv rules; + // its executable token must match the native drive-path representation. + quoteWindowsProcessArgument(command.replaceAll("/", "\\")), ...arguments_.map((value) => verbatim ? value : quoteWindowsProcessArgument(value), ),