fix(security): block cross-origin mutations on loopback browser routes
This commit is contained in:
@@ -5,6 +5,7 @@ import type { ResolvedBrowserConfig } from "./config.js";
|
||||
import type { BrowserRouteRegistrar } from "./routes/types.js";
|
||||
import { isLoopbackHost } from "../gateway/net.js";
|
||||
import { deleteBridgeAuthForPort, setBridgeAuthForPort } from "./bridge-auth-registry.js";
|
||||
import { browserMutationGuardMiddleware } from "./csrf.js";
|
||||
import { isAuthorizedBrowserRequest } from "./http-auth.js";
|
||||
import { registerBrowserRoutes } from "./routes/index.js";
|
||||
import {
|
||||
@@ -49,6 +50,7 @@ export async function startBrowserBridgeServer(params: {
|
||||
next();
|
||||
});
|
||||
app.use(express.json({ limit: "1mb" }));
|
||||
app.use(browserMutationGuardMiddleware());
|
||||
|
||||
const authToken = params.authToken?.trim() || undefined;
|
||||
const authPassword = params.authPassword?.trim() || undefined;
|
||||
|
||||
80
src/browser/csrf.test.ts
Normal file
80
src/browser/csrf.test.ts
Normal file
@@ -0,0 +1,80 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { shouldRejectBrowserMutation } from "./csrf.js";
|
||||
|
||||
describe("browser CSRF loopback mutation guard", () => {
|
||||
it("rejects mutating methods from non-loopback origin", () => {
|
||||
expect(
|
||||
shouldRejectBrowserMutation({
|
||||
method: "POST",
|
||||
origin: "https://evil.example",
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("allows mutating methods from loopback origin", () => {
|
||||
expect(
|
||||
shouldRejectBrowserMutation({
|
||||
method: "POST",
|
||||
origin: "http://127.0.0.1:18789",
|
||||
}),
|
||||
).toBe(false);
|
||||
|
||||
expect(
|
||||
shouldRejectBrowserMutation({
|
||||
method: "POST",
|
||||
origin: "http://localhost:18789",
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("allows mutating methods without origin/referer (non-browser clients)", () => {
|
||||
expect(
|
||||
shouldRejectBrowserMutation({
|
||||
method: "POST",
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects mutating methods with origin=null", () => {
|
||||
expect(
|
||||
shouldRejectBrowserMutation({
|
||||
method: "POST",
|
||||
origin: "null",
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects mutating methods from non-loopback referer", () => {
|
||||
expect(
|
||||
shouldRejectBrowserMutation({
|
||||
method: "POST",
|
||||
referer: "https://evil.example/attack",
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects cross-site mutations via Sec-Fetch-Site when present", () => {
|
||||
expect(
|
||||
shouldRejectBrowserMutation({
|
||||
method: "POST",
|
||||
secFetchSite: "cross-site",
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("does not reject non-mutating methods", () => {
|
||||
expect(
|
||||
shouldRejectBrowserMutation({
|
||||
method: "GET",
|
||||
origin: "https://evil.example",
|
||||
}),
|
||||
).toBe(false);
|
||||
|
||||
expect(
|
||||
shouldRejectBrowserMutation({
|
||||
method: "OPTIONS",
|
||||
origin: "https://evil.example",
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
83
src/browser/csrf.ts
Normal file
83
src/browser/csrf.ts
Normal file
@@ -0,0 +1,83 @@
|
||||
import type { NextFunction, Request, Response } from "express";
|
||||
import { isLoopbackHost } from "../gateway/net.js";
|
||||
|
||||
function firstHeader(value: string | string[] | undefined): string {
|
||||
return Array.isArray(value) ? (value[0] ?? "") : (value ?? "");
|
||||
}
|
||||
|
||||
function isMutatingMethod(method: string): boolean {
|
||||
const m = (method || "").trim().toUpperCase();
|
||||
return m === "POST" || m === "PUT" || m === "PATCH" || m === "DELETE";
|
||||
}
|
||||
|
||||
function isLoopbackUrl(value: string): boolean {
|
||||
const v = value.trim();
|
||||
if (!v || v === "null") {
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
const parsed = new URL(v);
|
||||
return isLoopbackHost(parsed.hostname);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
export function shouldRejectBrowserMutation(params: {
|
||||
method: string;
|
||||
origin?: string;
|
||||
referer?: string;
|
||||
secFetchSite?: string;
|
||||
}): boolean {
|
||||
if (!isMutatingMethod(params.method)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Strong signal when present: browser says this is cross-site.
|
||||
// Avoid being overly clever with "same-site" since localhost vs 127.0.0.1 may differ.
|
||||
const secFetchSite = (params.secFetchSite ?? "").trim().toLowerCase();
|
||||
if (secFetchSite === "cross-site") {
|
||||
return true;
|
||||
}
|
||||
|
||||
const origin = (params.origin ?? "").trim();
|
||||
if (origin) {
|
||||
return !isLoopbackUrl(origin);
|
||||
}
|
||||
|
||||
const referer = (params.referer ?? "").trim();
|
||||
if (referer) {
|
||||
return !isLoopbackUrl(referer);
|
||||
}
|
||||
|
||||
// Non-browser clients (curl/undici/Node) typically send no Origin/Referer.
|
||||
return false;
|
||||
}
|
||||
|
||||
export function browserMutationGuardMiddleware() {
|
||||
return (req: Request, res: Response, next: NextFunction) => {
|
||||
// OPTIONS is used for CORS preflight. Even if cross-origin, the preflight isn't mutating.
|
||||
const method = (req.method || "").trim().toUpperCase();
|
||||
if (method === "OPTIONS") {
|
||||
return next();
|
||||
}
|
||||
|
||||
const origin = firstHeader(req.headers.origin);
|
||||
const referer = firstHeader(req.headers.referer);
|
||||
const secFetchSite = firstHeader(req.headers["sec-fetch-site"]);
|
||||
|
||||
if (
|
||||
shouldRejectBrowserMutation({
|
||||
method,
|
||||
origin,
|
||||
referer,
|
||||
secFetchSite,
|
||||
})
|
||||
) {
|
||||
res.status(403).send("Forbidden");
|
||||
return;
|
||||
}
|
||||
|
||||
next();
|
||||
};
|
||||
}
|
||||
@@ -5,6 +5,7 @@ import { loadConfig } from "../config/config.js";
|
||||
import { createSubsystemLogger } from "../logging/subsystem.js";
|
||||
import { resolveBrowserConfig, resolveProfile } from "./config.js";
|
||||
import { ensureBrowserControlAuth, resolveBrowserControlAuth } from "./control-auth.js";
|
||||
import { browserMutationGuardMiddleware } from "./csrf.js";
|
||||
import { ensureChromeExtensionRelayServer } from "./extension-relay.js";
|
||||
import { isAuthorizedBrowserRequest } from "./http-auth.js";
|
||||
import { isPwAiLoaded } from "./pw-ai-state.js";
|
||||
@@ -56,6 +57,7 @@ export async function startBrowserControlServerFromConfig(): Promise<BrowserServ
|
||||
next();
|
||||
});
|
||||
app.use(express.json({ limit: "1mb" }));
|
||||
app.use(browserMutationGuardMiddleware());
|
||||
|
||||
if (browserAuth.token || browserAuth.password) {
|
||||
app.use((req, res, next) => {
|
||||
|
||||
Reference in New Issue
Block a user