Skip to content
Draft
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
15 changes: 15 additions & 0 deletions src/server/csrf.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,21 @@ describe("CSRF", () => {
);
});

test("rejects CSRF tokens signed with a different secret", async () => {
let token = new CSRF({ cookie, secret: "my-secret" }).generate();

let formData = new FormData();
formData.set("csrf", token);

let headers = new Headers({
cookie: await cookie.serialize(token),
});

expect(
new CSRF({ cookie, secret: "different-secret" }).validate(formData, headers),
).rejects.toThrow(new CSRFError("tampered_token_in_cookie", "Tampered CSRF token in cookie."));
});

test("commits the token to a cookie", async () => {
let [token, cookieHeader] = await csrf.commitToken();
let parsedCookie = await cookie.parse(cookieHeader);
Expand Down
21 changes: 17 additions & 4 deletions src/server/csrf.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,12 +173,24 @@
* @author [Sergio Xalambrí](https://sergiodxa.com)
* @module Server/CSRF
*/
import { sha256 } from "@oslojs/crypto/sha2";
import { hmac } from "@oslojs/crypto/hmac";
import { SHA256 } from "@oslojs/crypto/sha2";
import { encodeBase64url } from "@oslojs/encoding";
import type { Cookie } from "react-router";
import { randomString } from "../common/crypto.js";
import { getHeaders } from "./get-headers.js";

function timingSafeEqual(a: string, b: string) {
let mismatch = a.length ^ b.length;
let maxLength = Math.max(a.length, b.length);

for (let index = 0; index < maxLength; index++) {
mismatch |= (a.charCodeAt(index) || 0) ^ (b.charCodeAt(index) || 0);
}

return mismatch === 0;
}

export type CSRFErrorCode =
| "missing_token_in_cookie"
| "invalid_token_in_cookie"
Expand Down Expand Up @@ -343,14 +355,15 @@ export class CSRF {

private sign(token: string) {
if (!this.secret) return token;
return encodeBase64url(sha256(new TextEncoder().encode(token)));
let encoder = new TextEncoder();
return encodeBase64url(hmac(SHA256, encoder.encode(this.secret), encoder.encode(token)));
Comment on lines +358 to +359
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

verifySignature compares the provided signature to the expected signature using plain string equality. For authentication/verification primitives like HMAC, this can leak information via timing differences. Consider switching to a timing-safe comparison (e.g., compare decoded bytes in constant time, or reuse the repo’s timing-safe helper pattern) when checking the signature.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied in c96353b. CSRF signature verification now uses a timing-safe comparison, and I also fixed the CSRF test formatting so the Code Quality check passes.

}

private verifySignature(token: string) {
if (!this.secret) return true;
let [value, signature] = token.split(".");
if (!value) return false;
if (!value || !signature) return false;
let expectedSignature = this.sign(value);
return signature === expectedSignature;
return timingSafeEqual(signature, expectedSignature);
}
}
Loading