From b09c6b075228a285d3c047119addafde8aed368e Mon Sep 17 00:00:00 2001 From: Adolanium <94890352+Adolanium@users.noreply.github.com> Date: Sat, 24 Oct 2026 11:10:18 +0300 Subject: [PATCH] fix(apple): keep dyld candidates inside the analyzed root on Windows (#6955) FilesystemTreeView mapped root-relative dyld paths to host paths with path.join. On Windows a component such as ..\outside\x.dylib was normalized out of the analyzed root, so a file outside it was opened, parsed and hashed. Components containing a backslash or colon are now absent on Windows, Windows link text is split on backslashes, and every host path is checked to stay inside the root before it is opened. --- docs/apple-application-analysis.md | 3 + src/artifacts/apple/DylibResolutionReader.ts | 47 ++++-- tests/boundary/mcp/dylibResolutionMcp.test.ts | 147 ++++++++++++++++++ 3 files changed, 187 insertions(+), 10 deletions(-) diff --git a/docs/apple-application-analysis.md b/docs/apple-application-analysis.md index ef0fe09ccd..f52c31dfa1 100644 --- a/docs/apple-application-analysis.md +++ b/docs/apple-application-analysis.md @@ -213,6 +213,9 @@ Resolution follows dyld: already loaded image reuses it. - Symlinks such as `Versions/Current` are followed inside the analyzed root. An escape through `..` or an absolute link is `escapes-target`. +- Dyld path components are POSIX names. On Windows hosts, link text uses `\` + as its separator, and a dyld component that contains `\` or `:` cannot name + a file in the analyzed root, so its candidate is `absent`. Each edge lists every candidate path tried, with its outcome: `resolved`, `absent`, `not-mach-o`, `malformed`, `unsupported` (recognized Mach-O outside diff --git a/src/artifacts/apple/DylibResolutionReader.ts b/src/artifacts/apple/DylibResolutionReader.ts index f839348386..3a6ccba6c8 100644 --- a/src/artifacts/apple/DylibResolutionReader.ts +++ b/src/artifacts/apple/DylibResolutionReader.ts @@ -30,6 +30,7 @@ import { ArtifactResourceScope } from "../ArtifactResourceScope.js"; import { assertArtifactReadActive } from "../ArtifactFileOperation.js"; const OPERATION = "trace_dylib_resolution"; +const WINDOWS = process.platform === "win32"; /** Concurrent four-byte magic reads while enumerating bundle roots. */ const MAGIC_READ_CONCURRENCY = 8; @@ -71,12 +72,7 @@ class FilesystemTreeView implements DylibTreeView { image(path: string): Promise { const cached = this.#images.get(path); if (cached !== undefined) return cached; - const pending = this.files - .image(join(this.root, path)) - .then(({ facts, identity }) => { - this.#identities.set(path, identity); - return facts; - }); + const pending = this.#readImage(path); this.#images.set(path, pending); return pending; } @@ -86,13 +82,44 @@ class FilesystemTreeView implements DylibTreeView { return this.#identities.get(path); } + /** + * Host path of a root-relative dyld path. The resolver passes only plain + * components, so a path that lands outside the root is a reader defect and + * is refused before any file is opened. + */ + hostPath(path: string): string { + const absolute = join(this.root, path); + const inside = relative(this.root, absolute); + if (inside === ".." || inside.startsWith(`..${sep}`) || isAbsolute(inside)) + throw new ArtifactReaderFailure( + "path", + `${path} maps outside the analyzed root`, + ); + return absolute; + } + + async #readImage(path: string): Promise { + const { facts, identity } = await this.files.image(this.hostPath(path)); + this.#identities.set(path, identity); + return facts; + } + async #readEntry(path: string): Promise { assertArtifactReadActive(this.signal); - const absolute = join(this.root, path); + // A dyld path component is a POSIX name. Windows reads `\` as a separator + // and `:` as a stream delimiter, so no file in this tree has that name. + if (WINDOWS && /[\\:]/u.test(path)) return undefined; + const absolute = this.hostPath(path); try { const metadata = await lstat(absolute); - if (metadata.isSymbolicLink()) - return { kind: "symlink", target: await readlink(absolute) }; + if (metadata.isSymbolicLink()) { + const target = await readlink(absolute); + // Windows link text separates components with `\`. + return { + kind: "symlink", + target: WINDOWS ? target.replaceAll("\\", "/") : target, + }; + } if (metadata.isDirectory()) return { kind: "directory" }; return metadata.isFile() ? { kind: "file" } : undefined; } catch (cause: unknown) { @@ -168,7 +195,7 @@ export const traceDylibResolution = async (options: { for (const { path } of trace.images) digests.set( path, - await files.sha256(join(root, path), view.identity(path)), + await files.sha256(view.hostPath(path), view.identity(path)), ); const targetDigest = digests.get(target) ?? diff --git a/tests/boundary/mcp/dylibResolutionMcp.test.ts b/tests/boundary/mcp/dylibResolutionMcp.test.ts index d97961a733..f7b0990d65 100644 --- a/tests/boundary/mcp/dylibResolutionMcp.test.ts +++ b/tests/boundary/mcp/dylibResolutionMcp.test.ts @@ -1,4 +1,5 @@ import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; +import { createHash } from "node:crypto"; import { chmod, mkdir, symlink, writeFile } from "node:fs/promises"; import { join } from "node:path"; @@ -247,6 +248,152 @@ it("traces a standalone Mach-O file whose name ends in .app", async () => { }); }); +/** Trace `app` in `analyzed/`, next to a Mach-O dylib in `outside/`. */ +const traceBesideOutside = async ( + commands: readonly Uint8Array[], + prepare: (analyzed: string) => Promise = async () => {}, +): Promise<{ + readonly trace: z.infer; + readonly outsideSha256: string; +}> => { + const parent = await createTestTempDirectory("rea-dylib-mcp-contained-"); + const analyzed = join(parent, "analyzed"); + await mkdir(analyzed); + await mkdir(join(parent, "outside")); + const outside = machoImage({ + fileType: FILE_TYPE.dylib, + commands: [ + buildVersionCommand(1), + dylibCommand(LC.ID_DYLIB, "/outside/secret.dylib"), + ], + }); + await writeFile(join(parent, "outside/secret.dylib"), outside); + await writeFile( + join(analyzed, "app"), + machoImage({ commands: [buildVersionCommand(1), ...commands] }), + ); + await prepare(analyzed); + let trace: z.infer | undefined; + await withClient(async (client) => { + const opened = await client.callTool({ + name: "open_binary", + arguments: { path: join(analyzed, "app") }, + }); + expect(opened.isError, JSON.stringify(opened)).not.toBe(true); + const called = await client.callTool({ + name: "trace_dylib_resolution", + arguments: {}, + }); + expect(called.isError, JSON.stringify(called.structuredContent)).not.toBe( + true, + ); + trace = dylibResolutionResultSchema.parse( + structuredResult(called.structuredContent), + ); + }); + if (trace === undefined) throw new Error("trace_dylib_resolution not called"); + return { + trace, + outsideSha256: createHash("sha256").update(outside).digest("hex"), + }; +}; + +// Windows reads `\` as a separator, but a dyld component is a POSIX name. +it.runIf(process.platform === "win32")( + "does not read outside the root through backslashes in Windows dyld names", + async () => { + const { trace, outsideSha256 } = await traceBesideOutside([ + dylibCommand(LC.LOAD_DYLIB, "@loader_path/..\\outside\\secret.dylib"), + rpathCommand("@loader_path/..\\outside"), + dylibCommand(LC.LOAD_DYLIB, "@rpath/secret.dylib"), + ]); + expect(trace.edges.map(({ candidates }) => candidates)).toEqual([ + [ + expect.objectContaining({ + path: "..\\outside\\secret.dylib", + outcome: "absent", + resolved_path: null, + }), + ], + [ + expect.objectContaining({ + path: "..\\outside/secret.dylib", + source: "rpath", + outcome: "absent", + resolved_path: null, + }), + ], + ]); + expect(trace.images.map(({ path }) => path)).toEqual(["app"]); + expect(JSON.stringify(trace)).not.toContain(outsideSha256); + }, +); + +it("follows relative symlinks only while they stay inside the root", async () => { + const { trace, outsideSha256 } = await traceBesideOutside( + [ + dylibCommand(LC.LOAD_DYLIB, "@loader_path/link/secret.dylib"), + dylibCommand(LC.LOAD_DYLIB, "@loader_path/sub/lib/libinner.dylib"), + ], + // Windows stores these relative link texts as `..\outside` and `..\lib`. + async (analyzed) => { + await symlink("../outside", join(analyzed, "link"), "dir"); + await mkdir(join(analyzed, "lib")); + await mkdir(join(analyzed, "sub")); + await symlink("../lib", join(analyzed, "sub/lib"), "dir"); + await writeFile( + join(analyzed, "lib/libinner.dylib"), + machoImage({ + fileType: FILE_TYPE.dylib, + commands: [buildVersionCommand(1)], + }), + ); + }, + ); + expect(trace.edges.map(({ candidates }) => candidates)).toEqual([ + [ + expect.objectContaining({ + path: "link/secret.dylib", + outcome: "escapes-target", + resolved_path: null, + }), + ], + [ + expect.objectContaining({ + path: "sub/lib/libinner.dylib", + outcome: "resolved", + resolved_path: "lib/libinner.dylib", + }), + ], + ]); + expect(trace.images.map(({ path }) => path)).toEqual([ + "app", + "lib/libinner.dylib", + ]); + expect(JSON.stringify(trace)).not.toContain(outsideSha256); +}); + +it.skipIf(process.platform === "win32")( + "resolves a POSIX file name that contains backslashes", + async () => { + const { trace } = await traceBesideOutside( + [dylibCommand(LC.LOAD_DYLIB, "@loader_path/..\\outside\\secret.dylib")], + (analyzed) => + writeFile( + join(analyzed, "..\\outside\\secret.dylib"), + machoImage({ + fileType: FILE_TYPE.dylib, + commands: [buildVersionCommand(1)], + }), + ), + ); + expect(trace.edges[0]?.resolution).toEqual({ + status: "resolved", + image: "..\\outside\\secret.dylib", + }); + }, +); + it.skipIf(process.platform === "win32" || process.getuid?.() === 0)( "keeps a permission denied while canonicalizing the opened bundle", async () => {