diff --git a/src/utils.url.ts b/src/utils.url.ts index 5a725353..5115173f 100644 --- a/src/utils.url.ts +++ b/src/utils.url.ts @@ -36,6 +36,12 @@ export function joinURL(base?: string, path?: string): string { /** * Adds the base path to the input path, if it is not already present. + * + * The base-already-present short-circuit requires the character after the + * base to be a URL boundary (`/`, `?`, `#`) or end-of-string. Without the + * boundary check, an attacker-controlled `input` such as + * `http://api.internal.attacker.com/...` would be accepted as-if it already + * had the `http://api.internal` base and skip the join (CWE-918 SSRF). */ export function withBase(input = "", base = ""): string { if (!base || base === "/") { @@ -44,7 +50,15 @@ export function withBase(input = "", base = ""): string { const _base = withoutTrailingSlash(base); if (input.startsWith(_base)) { - return input; + const nextChar = input[_base.length]; + if ( + nextChar === undefined || + nextChar === "/" || + nextChar === "?" || + nextChar === "#" + ) { + return input; + } } return joinURL(_base, input); diff --git a/test/utils.url.test.ts b/test/utils.url.test.ts new file mode 100644 index 00000000..01b30a4b --- /dev/null +++ b/test/utils.url.test.ts @@ -0,0 +1,53 @@ +import { describe, it, expect } from "vitest"; +import { withBase } from "../src/utils.url.ts"; + +describe("withBase", () => { + it("returns the input unchanged when base is empty", () => { + expect(withBase("/foo", "")).toBe("/foo"); + }); + + it("returns the input unchanged when base is '/'", () => { + expect(withBase("/foo", "/")).toBe("/foo"); + }); + + it("joins base and input when input lacks the base", () => { + expect(withBase("/foo", "https://api.example.com")).toBe( + "https://api.example.com/foo" + ); + }); + + it("returns input unchanged when input already starts with base and the boundary is '/'", () => { + expect( + withBase("https://api.example.com/x", "https://api.example.com") + ).toBe("https://api.example.com/x"); + }); + + it("returns input unchanged when input equals base exactly", () => { + expect(withBase("https://api.example.com", "https://api.example.com")).toBe( + "https://api.example.com" + ); + }); + + it("returns input unchanged when boundary is '?'", () => { + expect( + withBase("https://api.example.com?q=1", "https://api.example.com") + ).toBe("https://api.example.com?q=1"); + }); + + it("returns input unchanged when boundary is '#'", () => { + expect( + withBase("https://api.example.com#frag", "https://api.example.com") + ).toBe("https://api.example.com#frag"); + }); + + // Regression test for issue #564: previously, an input like + // "http://api.internal.attacker.com/steal" would be returned unchanged + // when base was "http://api.internal", because it merely starts with the + // base string. That bypass let attacker-controlled inputs reach arbitrary + // hosts. The fix requires a URL boundary character after the base. + it("rejects boundary-less prefix match and joins instead (SSRF guard)", () => { + expect( + withBase("http://api.internal.attacker.com/steal", "http://api.internal") + ).toBe("http://api.internal/http://api.internal.attacker.com/steal"); + }); +});