diff --git a/.changeset/cyan-laws-mate.md b/.changeset/cyan-laws-mate.md new file mode 100644 index 000000000000..80cf99942b18 --- /dev/null +++ b/.changeset/cyan-laws-mate.md @@ -0,0 +1,5 @@ +--- +"@cloudflare/workers-shared": minor +--- + +Prevent same-schema attacks diff --git a/fixtures/asset-config/redirects.test.ts b/fixtures/asset-config/redirects.test.ts new file mode 100644 index 000000000000..5d724394e6c2 --- /dev/null +++ b/fixtures/asset-config/redirects.test.ts @@ -0,0 +1,89 @@ +import { SELF } from "cloudflare:test"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { applyConfigurationDefaults } from "../../packages/workers-shared/asset-worker/src/configuration"; +import Worker from "../../packages/workers-shared/asset-worker/src/index"; +import { getAssetWithMetadataFromKV } from "../../packages/workers-shared/asset-worker/src/utils/kv"; +import type { AssetMetadata } from "../../packages/workers-shared/asset-worker/src/utils/kv"; + +const IncomingRequest = Request; + +vi.mock("../../packages/workers-shared/asset-worker/src/utils/kv.ts"); +vi.mock("../../packages/workers-shared/asset-worker/src/configuration"); +const existsMock = (fileList: Set) => { + vi.spyOn(Worker.prototype, "unstable_exists").mockImplementation( + async (pathname: string) => { + if (fileList.has(pathname)) { + return pathname; + } + } + ); +}; +const BASE_URL = "http://example.com"; + +describe("[Asset Worker] `test url normalization`", () => { + afterEach(() => { + vi.mocked(getAssetWithMetadataFromKV).mockRestore(); + }); + beforeEach(() => { + vi.mocked(getAssetWithMetadataFromKV).mockImplementation( + () => + Promise.resolve({ + value: "no-op", + metadata: { + contentType: "no-op", + }, + }) as unknown as Promise< + KVNamespaceGetWithMetadataResult + > + ); + + vi.mocked(applyConfigurationDefaults).mockImplementation(() => { + return { + html_handling: "none", + not_found_handling: "none", + }; + }); + }); + + it("returns 404 for non matched encoded url", async () => { + const files = ["/christmas/starts/november/first.html"]; + const requestPath = "/%2f%2fbad.example.com%2f"; + + existsMock(new Set(files)); + const request = new IncomingRequest(BASE_URL + requestPath); + let response = await SELF.fetch(request, { redirect: "manual" }); + expect(response.status).toBe(404); + }); + + it("returns 200 for matched non encoded url", async () => { + const files = ["/you/lost/the/game.bin"]; + const requestPath = "/you/lost/the/game.bin"; + + existsMock(new Set(files)); + const request = new IncomingRequest(BASE_URL + requestPath); + let response = await SELF.fetch(request, { redirect: "manual" }); + expect(response.status).toBe(200); + }); + + it("returns redirect for matched encoded url", async () => { + const files = ["/awesome/file.bin"]; + const requestPath = "/awesome/file%2ebin"; + const finalPath = "/awesome/file.bin"; + + existsMock(new Set(files)); + const request = new IncomingRequest(BASE_URL + requestPath); + let response = await SELF.fetch(request, { redirect: "manual" }); + expect(response.status).toBe(307); + expect(response.headers.get("location")).toBe(finalPath); + }); + + it("returns 200 for matched non encoded url", async () => { + const files = ["/mylittlepony.png"]; + const requestPath = "/mylittlepony.png"; + + existsMock(new Set(files)); + const request = new IncomingRequest(BASE_URL + requestPath); + let response = await SELF.fetch(request, { redirect: "manual" }); + expect(response.status).toBe(200); + }); +}); diff --git a/fixtures/asset-config/url-normalization.test.ts b/fixtures/asset-config/url-normalization.test.ts new file mode 100644 index 000000000000..c8c716b35f54 --- /dev/null +++ b/fixtures/asset-config/url-normalization.test.ts @@ -0,0 +1,77 @@ +import { SELF } from "cloudflare:test"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { applyConfigurationDefaults } from "../../packages/workers-shared/asset-worker/src/configuration"; +import Worker from "../../packages/workers-shared/asset-worker/src/index"; +import { getAssetWithMetadataFromKV } from "../../packages/workers-shared/asset-worker/src/utils/kv"; +import type { AssetMetadata } from "../../packages/workers-shared/asset-worker/src/utils/kv"; + +const IncomingRequest = Request; + +vi.mock("../../packages/workers-shared/asset-worker/src/utils/kv.ts"); +vi.mock("../../packages/workers-shared/asset-worker/src/configuration"); +const existsMock = (fileList: Set) => { + vi.spyOn(Worker.prototype, "unstable_exists").mockImplementation( + async (pathname: string) => { + if (fileList.has(pathname)) { + return pathname; + } + } + ); +}; +const BASE_URL = "http://example.com"; + +describe("[Asset Worker] `test redirects`", () => { + afterEach(() => { + vi.mocked(getAssetWithMetadataFromKV).mockRestore(); + }); + beforeEach(() => { + vi.mocked(getAssetWithMetadataFromKV).mockImplementation( + () => + Promise.resolve({ + value: "no-op", + metadata: { + contentType: "no-op", + }, + }) as unknown as Promise< + KVNamespaceGetWithMetadataResult + > + ); + + vi.mocked(applyConfigurationDefaults).mockImplementation(() => { + return { + html_handling: "none", + not_found_handling: "none", + }; + }); + }); + + it("returns 200 leading encoded double slash", async () => { + const files = ["/blog/index.html"]; + const requestPath = "/%2fblog/index.html"; + + existsMock(new Set(files)); + const request = new IncomingRequest(BASE_URL + requestPath); + let response = await SELF.fetch(request); + expect(response.status).toBe(200); + }); + + it("returns 200 leading non encoded double slash", async () => { + const files = ["/blog/index.html"]; + const requestPath = "//blog/index.html"; + + existsMock(new Set(files)); + const request = new IncomingRequest(BASE_URL + requestPath); + let response = await SELF.fetch(request); + expect(response.status).toBe(200); + }); + + it("returns 404 for non matched url", async () => { + const files = ["/blog/index.html"]; + const requestPath = "/%2fexample.com/"; + + existsMock(new Set(files)); + const request = new IncomingRequest(BASE_URL + requestPath); + let response = await SELF.fetch(request); + expect(response.status).toBe(404); + }); +}); diff --git a/packages/workers-shared/asset-worker/src/handler.ts b/packages/workers-shared/asset-worker/src/handler.ts index 111279afa2ba..6e78aaf9cc58 100644 --- a/packages/workers-shared/asset-worker/src/handler.ts +++ b/packages/workers-shared/asset-worker/src/handler.ts @@ -18,7 +18,10 @@ export const handleRequest = async ( ) => { const { pathname, search } = new URL(request.url); - const decodedPathname = decodePath(pathname); + let decodedPathname = decodePath(pathname); + // normalize the path; remove multiple slashes which could lead to same-schema redirects + decodedPathname = decodedPathname.replace(/\/+/g, "/"); + const intent = await getIntent(decodedPathname, configuration, exists); if (!intent) { @@ -41,7 +44,7 @@ export const handleRequest = async ( * Thus we need to redirect if that is different from the decoded version. * We combine this with other redirects (e.g. for html_handling) to avoid multiple redirects. */ - if (encodedDestination !== pathname || intent.redirect) { + if ((encodedDestination !== pathname && intent.asset) || intent.redirect) { return new TemporaryRedirectResponse(encodedDestination + search); }