Skip to content

fix(csrf): remove hardcoded fallback secret and static anonymous session#1599

Open
Pcmhacker-piro wants to merge 1 commit into
RatLoopz:mainfrom
Pcmhacker-piro:fix/csrf-secret
Open

fix(csrf): remove hardcoded fallback secret and static anonymous session#1599
Pcmhacker-piro wants to merge 1 commit into
RatLoopz:mainfrom
Pcmhacker-piro:fix/csrf-secret

Conversation

@Pcmhacker-piro

Copy link
Copy Markdown
Contributor

Closes #1564

Changes

  • Remove fallback: getSecret now throws if CSRF_SECRET env var is not set, instead of falling back to a hardcoded string
  • Cryptographic session ID: Anonymous users get a crypto.randomUUID() stored in an httpOnly csrf_anon_id cookie, instead of the static "anonymous-session" value

Security

  • Prevents attackers from guessing the CSRF secret (previously "fallback-secret-change-in-production")
  • Prevents attackers from forging CSRF tokens for anonymous sessions (previously all anonymous sessions shared the same identifier)

- Replace fallback CSRF_SECRET with a thrown error when missing
- Replace static 'anonymous-session' with cryptographically random UUID
  stored in a session cookie for cross-request consistency
- Set httpOnly csrf_anon_id cookie on first CSRF token fetch
@dipexplorer

Copy link
Copy Markdown
Member

There is a subtle Express request lifecycle bug in the /api/csrf-token endpoint that will break validation for all anonymous users.

The Issue: When u call res.cookie(ANON_SESSION_COOKIE, anonId), it queues the cookie for the response headers, but it does not update req.cookies. When generateToken(req, res) runs on the very next line, getSessionIdentifier still sees req.cookies[ANON_SESSION_COOKIE] as undefined, causing it to generate a second, completely different randomUUID(). Result: The token is signed with a different UUID than the one sent to the browser!

The Fix: You just need to mutate the req.cookies object explicitly so generateToken reads the exact same UUID you just created.

if (!req.cookies?.[ANON_SESSION_COOKIE] && !req.cookies?.access_token) {
        const anonId = crypto.randomUUID();
        
        // FIX: Mutate req.cookies so generateToken binds to this exact ID
        if (!req.cookies) req.cookies = {};
        req.cookies[ANON_SESSION_COOKIE] = anonId; 
        res.cookie(ANON_SESSION_COOKIE, anonId, {
            // ... your existing config
        });
    }

Add this one-line fix, and we are ready to merge this security patch!

@dipexplorer dipexplorer added level:critical 80 pts quality:clean multiplier x1.2 and removed level:intermediate 35 pts labels Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gssoc:approved Approved for gssoc level:critical 80 pts quality:clean multiplier x1.2 type:security Auth, rate limiting, security

Projects

Status: 📥 Backlog

Development

Successfully merging this pull request may close these issues.

[HIGH] Hardcoded CSRF fallback secret and anonymous session — token forgery risk

2 participants