diff --git a/src/application/JavaScriptArtifactPathResolution.ts b/src/application/JavaScriptArtifactPathResolution.ts index c30c04fe78..8a165eb7b8 100644 --- a/src/application/JavaScriptArtifactPathResolution.ts +++ b/src/application/JavaScriptArtifactPathResolution.ts @@ -108,13 +108,19 @@ const rejectDeclaration = ( return unresolvedOutcome(input, "rejected", [ "NUL and backslash path syntax are not admitted for canonical artifact paths.", ]); - if (/%(?:2e|2f|5c)/iu.test(declared)) + if (!admitsCanonicalPathSyntax(declared)) return unresolvedOutcome(input, "rejected", [ "Encoded dot or separator bytes are rejected before artifact path resolution.", ]); return null; }; +/** Path syntax admitted for a canonical artifact path. */ +const admitsCanonicalPathSyntax = (value: string): boolean => + !value.includes("\0") && + !value.includes("\\") && + !/%(?:2e|2f|5c)/iu.test(value); + const contextualCandidate = ( input: ResolveArtifactPathInput, ): string | ArtifactPathResolution => { @@ -206,25 +212,54 @@ const htmlCandidate = ( input: ResolveArtifactPathInput, ): string | ArtifactPathResolution => { const declared = stripQueryAndFragment(input.declaredPath); - if (hasScheme(declared)) + if (hasScheme(declared) || declared.startsWith("//")) return unresolvedOutcome(input, "external", [ "External HTML references are not mapped to local artifact assets.", ]); - if (declared.startsWith("/")) return declared.slice(1); - const base = input.htmlBaseHref; - if (base === undefined || base === null || base === "") - return posix.join(posix.dirname(input.sourcePath), declared); - if (hasScheme(base)) + const rawBase = input.htmlBaseHref; + const base = + rawBase === undefined || rawBase === null + ? rawBase + : stripQueryAndFragment(rawBase); + if ( + base !== undefined && + base !== null && + (hasScheme(base) || base.startsWith("//")) + ) return unresolvedOutcome(input, "external", [ "The document base href is external, so its script reference is not a local artifact path.", ]); + if (declared.startsWith("/")) return declared.slice(1); + if (base === undefined || base === null || base === "") + return posix.join(posix.dirname(input.sourcePath), declared); + // A local base href is a second untrusted path input; apply the same + // admission rules a declared path gets so it cannot smuggle traversal or + // separator syntax past canonicalization. + if (!admitsCanonicalPathSyntax(base)) + return unresolvedOutcome(input, "rejected", [ + "The document base href uses NUL, backslash, or encoded dot and separator bytes that are not admitted for canonical artifact paths.", + ]); const basePath = base.startsWith("/") ? base.slice(1) : posix.join(posix.dirname(input.sourcePath), base); - const baseDirectory = base.endsWith("/") ? basePath : posix.dirname(basePath); - return posix.join(baseDirectory, declared); + return posix.join(htmlBaseDirectory(base, basePath), declared); }; +/** + * The directory a relative HTML reference resolves against. A base path + * ending in "/" or in a "." or ".." segment is already a directory, because a + * relative reference resolves against the base URL's directory and those + * segments are dropped rather than stepped through. + */ +const htmlBaseDirectory = (base: string, basePath: string): string => + base.endsWith("/") || + base.endsWith("/.") || + base.endsWith("/..") || + base === "." || + base === ".." + ? basePath + : posix.dirname(basePath); + const confineCandidate = ( input: ResolveArtifactPathInput, candidate: string, diff --git a/src/application/JavaScriptHtmlBaseResolution.test.ts b/src/application/JavaScriptHtmlBaseResolution.test.ts new file mode 100644 index 0000000000..a5a9fe93cb --- /dev/null +++ b/src/application/JavaScriptHtmlBaseResolution.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, it } from "vitest"; +import { resolveArtifactPathByContext } from "./JavaScriptArtifactPathResolution.js"; +import type { JavaScriptArtifactFile } from "./JavaScriptArtifactFiles.js"; +const paths = [ + "renderer/index.html", + "assets/app.js", + "renderer/app.js", + "renderer/a/app.js", + "app.js", +]; +const files = new Map( + paths.map((path) => [ + path, + { + path, + container_sha256: "a".repeat(64), + sha256: "b".repeat(64), + bytes: 0, + inventory_artifact_id: path, + kind: "javascript", + unpacked: false, + text: { included: true, value: "" }, + }, + ]), +); +const resolve = (declaredPath: string, htmlBaseHref: string) => + resolveArtifactPathByContext({ + declaredPath, + htmlBaseHref, + sourcePath: "renderer/index.html", + context: "html-reference", + files, + }); +const documentUrl = "https://artifact.test/renderer/index.html"; +describe("HTML base href URL components", () => { + it.each([ + "/assets/?cache=/wrong/", + "/assets/#/wrong/", + "/assets/index.html?cache=/wrong/", + ])("ignores query and fragment path-looking characters in %s", (base) => { + expect( + new URL( + "app.js", + new URL(base, "https://artifact.test/renderer/index.html"), + ).pathname, + ).toBe("/assets/app.js"); + expect(resolve("app.js", base)).toMatchObject({ + resolution_status: "resolved", + resolved_path: "assets/app.js", + }); + }); + it.each(["https://external.test/", "//external.test/"])( + "keeps root-relative references external when their base is %s", + (base) => { + expect(resolve("/assets/app.js", base)).toMatchObject({ + resolution_status: "external", + resolved_path: null, + }); + }, + ); +}); +describe("HTML base href dot segments", () => { + it.each([ + [".", "renderer/app.js"], + ["./", "renderer/app.js"], + ["a/.", "renderer/a/app.js"], + ["a/..", "renderer/app.js"], + ["..", "app.js"], + ])( + "resolves %s against the document directory like the URL parser", + (base, expected) => { + // A relative reference resolves against the base URL's directory, which + // drops a trailing "." or ".." segment instead of stepping through it. + expect( + decodeURIComponent( + new URL("app.js", new URL(base, documentUrl)).pathname, + ).replace(/^\/+/, ""), + ).toBe(expected); + expect(resolve("app.js", base)).toMatchObject({ + resolution_status: "resolved", + resolved_path: expected, + }); + }, + ); +}); +describe("HTML base href path syntax", () => { + it.each(["%2e%2e/secret/", "/a/%2e%2e/", "/a\\b/", "a\0b"])( + "rejects base href %j with the same admission rules a declared path gets", + (base) => { + const outcome = resolve("app.js", base); + expect(outcome.resolution_status).toBe("rejected"); + expect(outcome.limitations[0]).toContain( + "not admitted for canonical artifact paths", + ); + }, + ); + it("still confines a base href that escapes the artifact root", () => { + expect(resolve("app.js", "/a/../../secret/")).toMatchObject({ + resolution_status: "rejected", + limitations: [ + "The resolved candidate escapes the canonical artifact root.", + ], + }); + }); + it("reports an external base href as external rather than malformed", () => { + expect(resolve("app.js", "https://external.test/%2e%2e/")).toMatchObject({ + resolution_status: "external", + resolved_path: null, + }); + }); +});