Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
164 changes: 164 additions & 0 deletions packages/backend/src/checks/duplicate-cookies/index.spec.ts
Original file line number Diff line number Diff line change
@@ -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<unknown[]> => {
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);
});
});
});
76 changes: 76 additions & 0 deletions packages/backend/src/checks/duplicate-cookies/index.ts
Original file line number Diff line number Diff line change
@@ -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<string, number>();
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,
};
});
3 changes: 3 additions & 0 deletions packages/backend/src/checks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -111,5 +113,6 @@ export const checks = [
sqlStatementInParams,
ssnDisclosureScan,
suspectTransformScan,
duplicateCookiesScan,
// mysqlTimeBased,
] as const;
8 changes: 8 additions & 0 deletions packages/backend/src/stores/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,10 @@ export class ConfigStore {
checkID: Checks.MISSING_CONTENT_TYPE,
enabled: true,
},
{
checkID: Checks.DUPLICATE_COOKIES,
enabled: false,
},
],
},
{
Expand Down Expand Up @@ -371,6 +375,10 @@ export class ConfigStore {
checkID: Checks.MISSING_CONTENT_TYPE,
enabled: true,
},
{
checkID: Checks.DUPLICATE_COOKIES,
enabled: true,
},
],
},
{
Expand Down
Loading