Skip to content

fix: use randomBytes() instead of Math.random() for OAuth state gener…#145

Merged
ShantKhatri merged 1 commit into
Dev-Card:mainfrom
MehtabSandhu11:fix/oauth-state-crypto-random
May 18, 2026
Merged

fix: use randomBytes() instead of Math.random() for OAuth state gener…#145
ShantKhatri merged 1 commit into
Dev-Card:mainfrom
MehtabSandhu11:fix/oauth-state-crypto-random

Conversation

@MehtabSandhu11
Copy link
Copy Markdown
Contributor

@MehtabSandhu11 MehtabSandhu11 commented May 18, 2026

Summary

Fixes the generateState() function in auth.ts and connect.ts by replacing the insecure Math.random() implementation with a cryptographically secure random value for the OAuth state parameter.

Problem

The previous implementation used:

function generateState(): string {
  return Math.random().toString(36).substring(2, 15);
}

Math.random() is not cryptographically secure and can produce predictable values. Since the OAuth state parameter is used to prevent CSRF attacks, predictable state values weaken this protection.

Fix

Replaced the implementation with randomBytes() from Node.js' built-in crypto module:

import { randomBytes } from 'crypto';

function generateState(): string {
  return randomBytes(32).toString('hex');
}

This generates 256 bits of cryptographically secure randomness, making the OAuth state parameter unpredictable and resistant to CSRF attacks.

Files Changed

  • apps/backend/src/routes/auth.ts
  • apps/backend/src/routes/connect.ts

Related

Closes #123

@Harxhit Harxhit added the gssoc:approved Required label for every approved PR. Gives the base +50 points and enables contribution tracking. label May 18, 2026
@Harxhit Harxhit requested a review from ShantKhatri May 18, 2026 17:42
@Harxhit
Copy link
Copy Markdown
Collaborator

Harxhit commented May 18, 2026

@ShantKhatri These changes look good to merge. Please review them and provide the final approval.

@ShantKhatri
Copy link
Copy Markdown
Contributor

Hi @MehtabSandhu11 , we appreciate your contribution and also the PR standards you followed. Once again thanks for the contribution.

@ShantKhatri ShantKhatri merged commit e43ca72 into Dev-Card:main May 18, 2026
1 check failed
@Harxhit Harxhit added quality:clean PR is well-structured, readable, and follows good practices. (×1.2 multiplier) type:security Security-related fixes/improvements (+20 pts) level:intermediate Moderate difficulty contribution requiring some project understanding. (+35 pts) level:advanced Complex contribution involving deeper technical work. (+55 pts) and removed level:intermediate Moderate difficulty contribution requiring some project understanding. (+35 pts) labels May 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gssoc:approved Required label for every approved PR. Gives the base +50 points and enables contribution tracking. level:advanced Complex contribution involving deeper technical work. (+55 pts) quality:clean PR is well-structured, readable, and follows good practices. (×1.2 multiplier) type:security Security-related fixes/improvements (+20 pts)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[security] OAuth state parameter uses Math.random() — cryptographically weak, vulnerable to CSRF

3 participants