From a70ce7a0a2ba80c8d9da31c5d0acb0643c7f9199 Mon Sep 17 00:00:00 2001 From: George Oastler Date: Thu, 24 Sep 2026 23:57:18 +0100 Subject: [PATCH] Require the body site key to match the checked header domainMiddleware checks the origin allowlist of the prosopo-site-key header, but the handlers use the dapp in the body. Reject requests where the two differ, so a site's allowlist cannot be sidestepped by sending another key in the header (including a reserved test key). --- .../client-body-site-key-matches-header.md | 5 + packages/provider/src/api/domainMiddleware.ts | 12 ++ .../unit/api/domainMiddleware.unit.test.ts | 129 ++++++++++++++++++ 3 files changed, 146 insertions(+) create mode 100644 .changeset/client-body-site-key-matches-header.md create mode 100644 packages/provider/src/tests/unit/api/domainMiddleware.unit.test.ts diff --git a/.changeset/client-body-site-key-matches-header.md b/.changeset/client-body-site-key-matches-header.md new file mode 100644 index 0000000000..899077a009 --- /dev/null +++ b/.changeset/client-body-site-key-matches-header.md @@ -0,0 +1,5 @@ +--- +"@prosopo/provider": patch +--- + +Client captcha requests are now refused when the site key in the request body differs from the `prosopo-site-key` header. The allowed-domain check reads the header, but the captcha handlers act on the site key in the body, so a page on one site could send its own site key in the header and another site's key in the body and get that other site's challenges without its domain being allowed. The same gap let the reserved test site keys, which skip the domain check, carry any site's key in the body. diff --git a/packages/provider/src/api/domainMiddleware.ts b/packages/provider/src/api/domainMiddleware.ts index 9a050895a5..1ed86e0f2f 100644 --- a/packages/provider/src/api/domainMiddleware.ts +++ b/packages/provider/src/api/domainMiddleware.ts @@ -63,6 +63,13 @@ export const domainMiddleware = (env: ProviderEnvironment) => { throw invalidSiteKeyError(req.i18n, siteKey, req.logger); } + // Handlers act on the site key in the body, so it must be the one + // whose origin allowlist is checked here. Otherwise a request can + // pass as the caller's own site while acting for another. + const bodySiteKey = bodyDapp(req.body); + if (bodySiteKey !== undefined && bodySiteKey !== siteKey) + throw invalidSiteKeyError(req.i18n, siteKey, req.logger); + // Reserved CI test site keys have no DB record and no domain // allowlist; let them through so the deterministic test flow works in // every environment and from any origin. @@ -118,6 +125,11 @@ export const domainMiddleware = (env: ProviderEnvironment) => { }; }; +const bodyDapp = (body: unknown): unknown => + typeof body === "object" && body !== null && "dapp" in body + ? body.dapp + : undefined; + // If the origin is a Google Translate proxy URL, return the decoded original // origin(s) so the domain check can still match the site's allowed domains. const googleTranslateOrigins = (origin: string): string[] => { diff --git a/packages/provider/src/tests/unit/api/domainMiddleware.unit.test.ts b/packages/provider/src/tests/unit/api/domainMiddleware.unit.test.ts new file mode 100644 index 0000000000..4f7919dcd0 --- /dev/null +++ b/packages/provider/src/tests/unit/api/domainMiddleware.unit.test.ts @@ -0,0 +1,129 @@ +// Copyright 2021-2026 Prosopo (UK) Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import type { Server } from "node:http"; +import type { AddressInfo } from "node:net"; +import { i18nMiddleware } from "@prosopo/locale"; +import { getLogger } from "@prosopo/logger"; +import { ALWAYS_PASS_SITE_KEY } from "@prosopo/types"; +import type { ProviderEnvironment } from "@prosopo/types-env"; +import express, { type Request, type Response } from "express"; +import { + afterAll, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, +} from "vitest"; +import { domainMiddleware } from "../../../api/domainMiddleware.js"; + +const { getClientRecord } = vi.hoisted(() => ({ + getClientRecord: + vi.fn< + ( + siteKey: string, + ) => Promise<{ settings: { domains: string[] } } | undefined> + >(), +})); + +vi.mock("../../../tasks/index.js", () => ({ + Tasks: class { + db = { getClientRecord }; + clientTaskManager = { + domainPatternMatcher: (referrer: string, domain: string): boolean => + new URL(referrer).hostname === domain, + }; + }, +})); + +// Two valid sr25519 addresses: the attacker's own site key and a victim's. +const ATTACKER = "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY"; +const VICTIM = "5FHneW46xGXgs5mUiveU4sbTyGBzmstUspZC92UhjJM694ty"; + +describe("domainMiddleware", () => { + let server: Server; + let baseUrl: string; + + beforeAll(async () => { + const app = express(); + app.use(express.json()); + app.use((req: Request, _res: Response, next) => { + req.logger = getLogger("error", "test:domain-middleware"); + next(); + }); + app.use(await i18nMiddleware({})); + const env: Partial = {}; + app.use(domainMiddleware(env as ProviderEnvironment)); + app.post("/client", (_req: Request, res: Response) => { + res.json({ reached: true }); + }); + server = app.listen(0, "127.0.0.1"); + await new Promise((resolve) => server.once("listening", resolve)); + const address: AddressInfo | string | null = server.address(); + if (address === null || typeof address === "string") + throw new Error("test server has no TCP address"); + baseUrl = `http://127.0.0.1:${address.port}`; + }); + + afterAll(async () => { + await new Promise((resolve) => server.close(() => resolve())); + }); + + beforeEach(() => { + getClientRecord.mockReset(); + getClientRecord.mockImplementation(async (siteKey: string) => + siteKey === ATTACKER + ? { settings: { domains: ["attacker.example"] } } + : { settings: { domains: ["victim.example"] } }, + ); + }); + + const post = ( + headerSiteKey: string, + body: object, + ): Promise => + fetch(`${baseUrl}/client`, { + method: "POST", + headers: { + "content-type": "application/json", + origin: "https://attacker.example", + "prosopo-site-key": headerSiteKey, + }, + body: JSON.stringify(body), + }); + + it("lets a request through when the body site key matches the header", async () => { + const res = await post(ATTACKER, { dapp: ATTACKER }); + expect(res.status).toBe(200); + }); + + it("rejects a body site key that differs from the origin-checked header", async () => { + // The attacker's origin is allowed for their own key only; the handler + // would act for the victim's key taken from the body. + const res = await post(ATTACKER, { dapp: VICTIM }); + expect(res.status).toBe(400); + }); + + it("rejects a victim body site key behind a reserved test site key header", async () => { + const res = await post(ALWAYS_PASS_SITE_KEY, { dapp: VICTIM }); + expect(res.status).toBe(400); + }); + + it("lets a request with no body site key through on the header alone", async () => { + const res = await post(ATTACKER, {}); + expect(res.status).toBe(200); + }); +});