diff --git a/packages/backend/src/checks/duplicate-cookies/index.spec.ts b/packages/backend/src/checks/duplicate-cookies/index.spec.ts new file mode 100644 index 0000000..b958acb --- /dev/null +++ b/packages/backend/src/checks/duplicate-cookies/index.spec.ts @@ -0,0 +1,164 @@ +import { createMockRequest, createMockResponse, runCheck } from "engine"; +import { describe, expect, it } from "vitest"; + +import duplicateCookiesCheck from "./index"; + +const runDuplicateCookieCheck = async ( + setCookieHeaders: string[], +): Promise => { + const request = createMockRequest({ + id: "req", + host: "example.com", + method: "GET", + path: "/", + }); + + const response = createMockResponse({ + id: "res", + code: 200, + headers: { "set-cookie": setCookieHeaders }, + body: "OK", + }); + + const execution = await runCheck(duplicateCookiesCheck, [ + { request, response }, + ]); + + return execution[0]?.steps[execution[0].steps.length - 1]?.findings ?? []; +}; + +describe("Duplicate cookies check", () => { + describe("Detection", () => { + it("should detect duplicate cookie names (case-insensitive)", async () => { + const findings = await runDuplicateCookieCheck([ + "sessionId=abc123; Path=/; HttpOnly", + "SessionID=def456; Path=/; Secure", + ]); + + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ + name: "Duplicate cookies set", + severity: "low", + }); + expect(findings[0].description).toContain("sessionid"); + expect(findings[0].description).toContain("2 times"); + }); + + it("should detect cookie set more than twice", async () => { + const findings = await runDuplicateCookieCheck([ + "token=aaa", + "Token=bbb", + "TOKEN=ccc", + ]); + + expect(findings).toHaveLength(1); + expect(findings[0].description).toContain("token"); + expect(findings[0].description).toContain("3 times"); + }); + + it("should detect multiple different cookies with duplicates", async () => { + const findings = await runDuplicateCookieCheck([ + "session=a", + "Session=b", + "user=x", + "User=y", + ]); + + expect(findings).toHaveLength(1); + expect(findings[0].description).toContain("session"); + expect(findings[0].description).toContain("user"); + }); + + it("should report only duplicated cookies, not unique ones", async () => { + const findings = await runDuplicateCookieCheck([ + "session=a", + "session=b", + "lang=en", + ]); + + expect(findings).toHaveLength(1); + expect(findings[0].description).toContain("session"); + expect(findings[0].description).not.toContain("lang"); + }); + }); + + describe("False Positives", () => { + it("should not flag unique cookie names", async () => { + const findings = await runDuplicateCookieCheck([ + "sessionId=abc123", + "lang=en", + "theme=dark", + ]); + + expect(findings).toHaveLength(0); + }); + + it("should not flag when no Set-Cookie headers present", async () => { + const request = createMockRequest({ + id: "req", + host: "example.com", + method: "GET", + path: "/", + }); + + const response = createMockResponse({ + id: "res", + code: 200, + headers: {}, + body: "OK", + }); + + const execution = await runCheck(duplicateCookiesCheck, [ + { request, response }, + ]); + + const findings = + execution[0]?.steps[execution[0].steps.length - 1]?.findings ?? []; + expect(findings).toHaveLength(0); + }); + + it("should not flag single cookie", async () => { + const findings = await runDuplicateCookieCheck(["sessionId=abc123"]); + expect(findings).toHaveLength(0); + }); + }); + + describe("Edge Cases", () => { + it("should not run when response is missing", async () => { + const request = createMockRequest({ + id: "req", + host: "example.com", + method: "GET", + path: "/", + }); + + const execution = await runCheck(duplicateCookiesCheck, [ + { request, response: undefined }, + ]); + + const findings = + execution[0]?.steps[execution[0].steps.length - 1]?.findings ?? []; + expect(findings).toHaveLength(0); + }); + + it("should include security guidance in description", async () => { + const findings = await runDuplicateCookieCheck([ + "session=a", + "session=b", + ]); + + expect(findings[0].description).toContain("session fixation"); + expect(findings[0].description).toContain("inconsistent behaviour"); + }); + + it("should normalize cookie names to lowercase", async () => { + const findings = await runDuplicateCookieCheck([ + "SeSsIoN=a", + "sEsSiOn=b", + ]); + + expect(findings).toHaveLength(1); + expect(findings[0].description).toMatch(/session.*2 times/i); + }); + }); +}); diff --git a/packages/backend/src/checks/duplicate-cookies/index.ts b/packages/backend/src/checks/duplicate-cookies/index.ts new file mode 100644 index 0000000..240857e --- /dev/null +++ b/packages/backend/src/checks/duplicate-cookies/index.ts @@ -0,0 +1,76 @@ +import { defineCheck, done, Severity } from "engine"; + +import { Tags } from "../../types"; +import { getSetCookieHeaders, keyStrategy } from "../../utils"; + +export default defineCheck(({ step }) => { + step("detectDuplicateCookies", (state, context) => { + const { response, request } = context.target; + + if (!response) { + return done({ state }); + } + + const cookies = getSetCookieHeaders(response); + if (cookies.length === 0) { + return done({ state }); + } + + const counts = new Map(); + for (const cookie of cookies) { + const key = cookie.key.toLowerCase(); + counts.set(key, (counts.get(key) ?? 0) + 1); + } + + const duplicates = Array.from(counts.entries()).filter( + ([, count]) => count > 1, + ); + if (duplicates.length === 0) { + return done({ state }); + } + + const details = duplicates + .map(([name, count]) => `- Cookie \`${name}\` is set ${count} times`) + .join("\n"); + + const description = [ + "The response sets the same cookie name multiple times.", + "", + details, + "", + "Browsers may choose an arbitrary value, enabling session fixation or inconsistent behaviour.", + "**Recommendation:** Ensure each cookie name is set only once per response and consolidate attributes if needed.", + ].join("\n"); + + return done({ + state, + findings: [ + { + name: "Duplicate cookies set", + description, + severity: Severity.LOW, + correlation: { + requestID: request.getId(), + locations: [], + }, + }, + ], + }); + }); + + return { + metadata: { + id: "duplicate-cookies", + name: "Duplicate cookies set", + description: + "Detects responses that set the same cookie name multiple times.", + type: "passive", + tags: [Tags.COOKIES, Tags.SECURITY_HEADERS], + severities: [Severity.LOW], + aggressivity: { minRequests: 0, maxRequests: 0 }, + }, + initState: () => ({}), + dedupeKey: keyStrategy().withHost().withPort().withPath().build(), + when: () => true, + }; +}); diff --git a/packages/backend/src/checks/index.ts b/packages/backend/src/checks/index.ts index 1c28c38..0d702b9 100644 --- a/packages/backend/src/checks/index.ts +++ b/packages/backend/src/checks/index.ts @@ -14,6 +14,7 @@ import cspUntrustedStyleScan from "./csp-untrusted-style"; import dbConnectionDisclosureScan from "./db-connection-disclosure"; import debugErrorsScan from "./debug-errors"; import directoryListingScan from "./directory-listing"; +import duplicateCookiesScan from "./duplicate-cookies"; import emailDisclosureScan from "./email-disclosure"; import exposedEnvScan from "./exposed-env"; import gitConfigScan from "./git-config"; @@ -71,6 +72,7 @@ export const Checks = { SQL_STATEMENT_IN_PARAMS: "sql-statement-in-params", SSN_DISCLOSURE: "ssn-disclosure", SUSPECT_TRANSFORM: "suspect-transform", + DUPLICATE_COOKIES: "duplicate-cookies", // MYSQL_TIME_BASED_SQLI: "mysql-time-based-sqli" - TODO: fix false positives } as const; @@ -111,5 +113,6 @@ export const checks = [ sqlStatementInParams, ssnDisclosureScan, suspectTransformScan, + duplicateCookiesScan, // mysqlTimeBased, ] as const; diff --git a/packages/backend/src/stores/config.ts b/packages/backend/src/stores/config.ts index 6b716b7..123915b 100644 --- a/packages/backend/src/stores/config.ts +++ b/packages/backend/src/stores/config.ts @@ -188,6 +188,10 @@ export class ConfigStore { checkID: Checks.MISSING_CONTENT_TYPE, enabled: true, }, + { + checkID: Checks.DUPLICATE_COOKIES, + enabled: false, + }, ], }, { @@ -371,6 +375,10 @@ export class ConfigStore { checkID: Checks.MISSING_CONTENT_TYPE, enabled: true, }, + { + checkID: Checks.DUPLICATE_COOKIES, + enabled: true, + }, ], }, {