fix(javascript): respect HTML base URL components (#589)
* fix(javascript): respect HTML base URL components Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com> * fix(javascript): resolve HTML base href dot segments and gate base syntax A relative HTML reference resolves against the base URL's directory, and that directory drops a trailing "." or ".." segment rather than stepping through it. Only a trailing "/" was treated as a directory, so <base href="."> resolved against the artifact root instead of the document directory, and <base href="sub/."> resolved against the parent of "sub". The base href is also a second untrusted path input, but rejectDeclaration only applied its admission rules to the declared path, so an encoded-traversal or backslash base href relied on files-map confinement to stay safe. Both inputs now share one admitsCanonicalPathSyntax predicate, and a local base href is rejected with a reason naming the base href rather than a generic declared-path message. External base hrefs are still reported as external, so an encoded path on an unreachable origin keeps the more accurate reason. 7 of the new cases fail without this change. * style: apply Prettier to the HTML base resolution changes --------- Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com> Co-authored-by: morluto <76467478+morluto@users.noreply.github.com>
This commit is contained in:
Rudy Mizrahi Celekli
co-authored by
morluto
morluto
parent
82957c98bf
commit
ffbde92f9a
@@ -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,
|
||||
|
||||
@@ -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<string, JavaScriptArtifactFile>(
|
||||
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,
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user