Insecure Randomness Code Review Guide
Table of Contents
Introduction
Most security bugs are unsafe inputs. Insecure randomness is the opposite: it is an unsafe output. The system is working exactly as its author intended, but the numbers it produces are predictable to anyone who understands how they were generated. Predictable numbers used as passwords, session IDs, password-reset tokens, crypto nonces, or CSRF tokens do not become any less predictable when wrapped in base64 or SHA-256 — they simply become predictable strings of base64 or hex.
The failure is usually not that a developer chose the wrong library. It is that they reached for the obvious function — Math.random(), rand(), java.util.Random, random.random() — without knowing that "random" in a programming language standard library almost always means "statistically random for simulations and games" and almost never means "unpredictable to an adversary." Those are two different specifications, and conflating them is the single root cause behind this entire bug class.
A 32-bit seed is not a secret
If a PRNG is seeded from a 32-bit value (a timestamp, a PID, an incrementing counter), the total keyspace is at most ~4 billion. That is trivially brute-forceable on a laptop in seconds, regardless of how long the token it produces looks. The security of the output is capped by the entropy of the seed, not the length of the string.
Two decades of real incidents make the point. In 2008, the Debian OpenSSL maintainer commented out a line that fed uninitialised memory into the PRNG, reducing the seed space to the PID (15 bits). Every SSH key, SSL certificate, and DNSSEC key generated on an affected Debian or Ubuntu system for the next 20 months was one of 32,768 possible keys. In 2010, a Hacker News contributor demonstrated that the site's session cookies were seeded from a millisecond timestamp and could be guessed within an hour. The fix in both cases was a one-line API swap — but the systems were built on the wrong abstraction from the start.
A reviewer sees `resetToken = Math.random().toString(36).slice(2)`. The token is 10 characters of base36 (~52 bits). Is that enough entropy?
PRNG vs CSPRNG
Every language ships at least two random-number APIs: one designed for speed and statistical quality (good enough to pass the dieharder test suite, fine for a Monte Carlo simulation or a video game), and one designed to be unpredictable to an adversary who observes outputs. They look almost identical at the call site, and the unsafe one is almost always the shorter name.
Statistical PRNG vs Cryptographic PRNG
Math.random()— JSrand() / random()— C, libcjava.util.Random— Javarandom.random()— Pythonmath/rand— Go (pre-1.20)
crypto.randomBytes— Nodewindow.crypto.getRandomValues— Browsersecrets— Python 3.6+java.security.SecureRandom— Javacrypto/rand— Go
/dev/urandom, getrandom(),BCryptGenRandom). State recovery is computationally infeasible.Rule of thumb: if a value ever gates access, identifies a user, signs something, or becomes a secret — it must come from the right-hand column. There is no middle ground.
Safe vs Unsafe Randomness APIs by Language
| Language | Unsafe (statistical) | Safe (cryptographic) |
|---|---|---|
| JavaScript (Node) | Math.random() | crypto.randomBytes, crypto.randomUUID, crypto.randomInt |
| JavaScript (Browser) | Math.random() | window.crypto.getRandomValues, crypto.randomUUID |
| Python | random module (random.random, random.choice, random.randint) | secrets module (secrets.token_urlsafe, secrets.token_hex, secrets.choice) |
| Java | java.util.Random, Math.random() | java.security.SecureRandom |
| Go | math/rand (pre-1.22), math/rand/v2 | crypto/rand |
| Ruby | rand, Random.new | SecureRandom (hex, urlsafe_base64, uuid) |
| PHP | rand, mt_rand | random_bytes, random_int |
| C / C++ | rand, random, drand48 | getrandom(2), arc4random_buf, BCryptGenRandom |
| .NET / C# | System.Random | RandomNumberGenerator.GetBytes, RandomNumberGenerator.GetInt32 |
| Rust | rand::thread_rng (unless seeded from OsRng) | rand::rngs::OsRng, getrandom crate |
The practical review move is simple: treat every use of the left column as a finding by default, and ask the author to justify why the output is not flowing into a security-sensitive sink. The burden of proof is on the unsafe API, not on the reviewer. This inverts the usual "innocent until proven guilty" and is the right default because the cost of a false positive (a conversation) is trivial compared to the cost of a false negative (an account-takeover CVE).
The Same Operation, Safe and Unsafe
1// UNSAFE — statistical PRNG (xorshift128+ in V8).
2// State is recoverable from ~5 consecutive outputs.
3const token = Math.random().toString(36).slice(2, 18);
4
5// SAFE — cryptographic PRNG, 128 bits of entropy.
6import { randomBytes } from 'node:crypto';
7const token = randomBytes(16).toString('base64url');
8
9// SAFE, short — 128-bit UUID v4 from the OS CSPRNG.
10import { randomUUID } from 'node:crypto';
11const id = randomUUID();
12
13// SAFE, integer — crypto.randomInt is unbiased by rejection sampling.
14import { randomInt } from 'node:crypto';
15const otp = randomInt(0, 1_000_000).toString().padStart(6, '0');Node’s crypto.randomUUID is faster than you think
Developers sometimes avoid crypto.randomUUID() in hot paths under the belief that it is slow. On modern Node it is within a factor of 2 of Math.random() on a single thread and trivially fast for any workload that is not generating millions of IDs per second per core. The performance argument is almost never load-bearing; the security argument always is.
In Python, `random.choice(string.ascii_letters + string.digits)` is called in a loop to build a 32-character API key. What is the correct review comment?
Predictable Seeds & State Recovery
A PRNG is a pure function from state to (state, output). Two generators in the same state will emit the same outputs forever. So the security of a PRNG-derived secret reduces to one question: how much entropy is in the initial state, and how easy is it for an attacker to reproduce it? When the seed is a timestamp, a process ID, a millisecond counter, or anything an attacker can observe or narrow down, the whole chain collapses.
How an Attacker Recovers PRNG State from a Few Outputs
srand(time(NULL)); // only ~2^31 possible seeds for a given minuteSignup token = abc123... // attacker signs up and reads their own tokenLCG: X = aX + c mod m — 2 outputs is enough; Mersenne Twister: 624 x 32 bitsGuess the next user's password-reset token. Or the previous one. Account takeover.Key Insight: A non-cryptographic PRNG is a deterministic function of its state. "It looks random in my tests" is not a property that matters — what matters is whether an adversary, given what they can observe, can compute future outputs faster than guessing.
Classic Weak Seeds (C, JavaScript, Java)
1/* C — the textbook anti-pattern. time(NULL) is a 32-bit second-resolution
2 timestamp. An attacker who knows roughly when a token was generated
3 has at most a few thousand candidate seeds to try. */
4#include <stdlib.h>
5#include <time.h>
6srand(time(NULL));
7int token = rand(); // fully determined by the second of issuance
8
9/* Slightly worse: seed the generator inside a loop. Every iteration in
10 the same second gets the SAME output. */
11for (int i = 0; i < 10; i++) {
12 srand(time(NULL));
13 printf("%d\n", rand()); // prints the same number 10 times
14}Recovering Mersenne Twister state in JavaScript
1// Math.random in V8 is xorshift128+ (not MT), but the same principle
2// applies: with enough observed outputs, the internal state is solvable.
3//
4// Proof-of-concept attack shape:
5// 1. Attacker signs up, receives a password-reset token minted from
6// Math.random().
7// 2. Attacker observes 5 consecutive 64-bit outputs from the same
8// process (e.g. by requesting 5 tokens back-to-back).
9// 3. Attacker solves the xorshift128+ recurrence for the internal
10// 64-bit state pair (s0, s1) using a SAT solver or z3. This is a
11// well-documented exercise; multiple open-source tools do it.
12// 4. Attacker rolls the state backwards or forwards and computes the
13// NEXT victim's reset token before they click the email link.
14//
15// Mitigation: do not generate security tokens from Math.random().
16// There is no quantity of hashing/encoding/length that fixes this.Seeding a CSPRNG from a weak source is still insecure
A developer once wrote SecureRandom sr = new SecureRandom(Long.toString(System.currentTimeMillis()).getBytes());. Even though the class name says "secure," the explicit seed constructor replaces the OS entropy with the caller-supplied bytes. The output is now deterministic from a millisecond timestamp. Always use SecureRandom.getInstanceStrong() or the no-arg constructor, which reseeds from the OS.
Java: the explicit-seed footgun
1// UNSAFE — replaces OS entropy with a timestamp. Output fully recoverable.
2SecureRandom sr = new SecureRandom(
3 Long.toString(System.currentTimeMillis()).getBytes());
4byte[] token = new byte[32];
5sr.nextBytes(token);
6
7// SAFE — no seed argument: the RNG reseeds from the OS on construction.
8SecureRandom sr = new SecureRandom();
9byte[] token = new byte[32];
10sr.nextBytes(token);
11
12// BEST — getInstanceStrong() picks the algorithm configured in
13// securerandom.strongAlgorithms for this platform. Blocks on
14// Linux if /dev/random is starved; use with care on boot.
15SecureRandom sr = SecureRandom.getInstanceStrong();A service generates 6-digit email verification codes using `Math.floor(Math.random() * 1_000_000)`. Logs show tokens issued at ~50 per second. Why is changing this to crypto.randomInt a higher-priority fix than increasing the length to 8 digits?
Security-Sensitive Sinks
Not every random number in a codebase is security-relevant. A jittered retry timeout, a shuffle in a game, a particle-effect seed — these are fine with Math.random(). The review job is to identify the sinks where the output flows into a security decision, and demand a cryptographic source there. The list below covers the sinks that account for almost every real-world CVE in this class.
Where Randomness Becomes Security
| Sink | Failure mode if predictable | Minimum entropy |
|---|---|---|
| Session IDs | Attacker predicts the next session ID and impersonates a user | 128 bits |
| Password-reset tokens | Attacker guesses a victim’s token before they click the email link | 128 bits |
| Email verification codes | Attacker bypasses email confirmation and activates attacker-owned accounts | 128 bits or strict rate-limit on short codes |
| CSRF tokens | Attacker forges a valid CSRF token and bypasses the protection entirely | 128 bits |
| OAuth state / PKCE verifier | CSRF on the authorization callback; PKCE verifier must be unguessable by a concurrent attacker | 128 bits (PKCE spec requires 43-128 char high-entropy string) |
| JWT jti / nonce claims | Replay protection fails; two tokens collide | 128 bits |
| Crypto IVs / nonces (AES-GCM, ChaCha20) | Nonce reuse with the same key breaks confidentiality AND integrity | 96 bits random OR a strict monotonic counter |
| TLS / SSH / signing keys | Key compromise, universal forgery | 256 bits seed material from OS CSPRNG |
| Temporary filenames (mktemp) | Symlink / race attacks if attacker predicts the path in a shared tmpdir | Use O_EXCL / mkstemp, do not roll your own |
| Captcha / OTP | Attacker predicts the challenge or bypass the second factor | 128 bits or short code + strict rate-limit + expiry |
| Password generators | Generated passwords are guessable by anyone who observes a few | 128+ bits from a CSPRNG |
The same sink, wrong and right
1// WRONG — password reset token from Math.random.
2function makeResetToken(): string {
3 return Math.random().toString(36).slice(2, 14);
4}
5
6// WRONG — session id from Date.now() + counter.
7let counter = 0;
8function makeSessionId(): string {
9 return (Date.now().toString(36) + (counter++).toString(36));
10}
11
12// WRONG — using Math.random for an AES-GCM IV. NONCE REUSE.
13const iv = Buffer.alloc(12);
14for (let i = 0; i < 12; i++) iv[i] = Math.floor(Math.random() * 256);
15
16// RIGHT — all three fixed with the standard library.
17import { randomBytes, randomUUID } from 'node:crypto';
18const resetToken = randomBytes(32).toString('base64url'); // 256 bits
19const sessionId = randomUUID(); // 122 bits random
20const iv = randomBytes(12); // 96-bit GCM nonceUUIDv1 is a timestamp, not a secret
UUIDv1 embeds a 60-bit timestamp and a MAC address. It is unique but not secret. Never use UUIDv1 where you need unpredictability — use UUIDv4 (122 random bits) or, better, crypto.randomBytes encoded to base64url. This is a surprisingly common confusion in Java codebases that reach for UUID.randomUUID() (v4, safe) vs UUIDs.timeBased() from Cassandra's driver (v1, not safe).
Which of the following is the ONLY acceptable randomness source for an AES-GCM initialization vector?
Bias & Modulo Reduction
Even when the underlying generator is a CSPRNG, the way a developer reduces its output to a smaller range can reintroduce bias. The classic bug is rand() % N: if N does not divide RAND_MAX + 1, the low residues are slightly more likely than the high ones. For most ranges this is a small statistical bias, but for short ranges — lottery picks, shuffled-deck positions, OTP digits — it is often enough to give an attacker a measurable edge.
The modulo bias, measured
1/* Suppose RAND_MAX = 32767 (a common libc value) and we do: rand() % 10
2 There are 32768 possible rand() outputs. 32768 / 10 = 3276 remainder 8.
3 So the residues 0..7 each occur 3277 times, and residues 8..9 each
4 occur 3276 times. Bias is small (~0.03%).
5
6 But try rand() % 30000 on the same RNG:
7 32768 / 30000 = 1 remainder 2768.
8 Residues 0..2767 each occur 2 times; residues 2768..29999 occur 1 time.
9 Residues 0..2767 are now roughly TWICE as likely as any other. */
10
11/* Correct: rejection sampling. */
12int randrange(int n) {
13 int limit = RAND_MAX - (RAND_MAX % n); // largest multiple of n <= RAND_MAX
14 int r;
15 do { r = rand(); } while (r >= limit);
16 return r % n;
17}Languages that give you a proper bounded-integer API hide this detail from you. Use them. crypto.randomInt(min, max) in Node, secrets.randbelow(n) in Python, java.security.SecureRandom.nextInt(int) in Java, rand.Int(rand.Reader, max) in Go — all of these perform the correct rejection sampling internally. Rolling your own with % is almost always wrong and almost never necessary.
Bounded integer APIs (safe)
1// Node.js — crypto.randomInt does rejection sampling under the hood.
2import { randomInt } from 'node:crypto';
3const diceRoll = randomInt(1, 7); // 1..6 inclusive, unbiased
4const otp = randomInt(0, 1_000_000); // 0..999999, unbiased
5
6// Python — secrets.randbelow(n) is the canonical API.
7import secrets
8dice_roll = secrets.randbelow(6) + 1
9otp = secrets.randbelow(1_000_000)
10
11// Java — SecureRandom.nextInt(bound) is unbiased.
12import java.security.SecureRandom;
13SecureRandom sr = new SecureRandom();
14int diceRoll = sr.nextInt(6) + 1;
15
16// Go — crypto/rand Int returns a bignum in [0, max).
17import ("crypto/rand"; "math/big")
18n, _ := rand.Int(rand.Reader, big.NewInt(6))
19diceRoll := int(n.Int64()) + 1Bias is a second-order concern — source is first
Modulo bias only matters if your source is already cryptographic. A biased OTP drawn from Math.random() has two problems, but the catastrophic one is the source. Fix the source first; then fix the reduction. A reviewer who flags % 1000000 on a CSPRNG but misses the same pattern on Math.random() has their priorities inverted.
A Node service generates 6-digit OTPs via `crypto.randomBytes(4).readUInt32BE(0) % 1_000_000`. Is that safe?
Prevention Basics
The good news about insecure randomness is that the fix is almost always a one-line API swap, and every modern language ships the right API in its standard library. The review rule you want your team to internalise is: the only acceptable source of security-relevant randomness is the language's documented CSPRNG API, called with no explicit seed.
Safe Defaults Per Language
| Need | Node / TS | Python | Java | Go | Ruby | PHP |
|---|---|---|---|---|---|---|
| Random bytes | crypto.randomBytes(n) | secrets.token_bytes(n) | new SecureRandom().nextBytes(b) | crypto/rand.Read(b) | SecureRandom.random_bytes(n) | random_bytes(n) |
| URL-safe token | crypto.randomBytes(32).toString('base64url') | secrets.token_urlsafe(32) | Base64.getUrlEncoder().encodeToString(bytes) | base64.RawURLEncoding.EncodeToString(b) | SecureRandom.urlsafe_base64(32) | rtrim(strtr(base64_encode(random_bytes(32)), '+/', '-_'), '=') |
| Bounded integer | crypto.randomInt(0, n) | secrets.randbelow(n) | new SecureRandom().nextInt(n) | rand.Int(rand.Reader, big.NewInt(n)) | SecureRandom.random_number(n) | random_int(0, n-1) |
| UUID v4 | crypto.randomUUID() | uuid.uuid4() | UUID.randomUUID() | github.com/google/uuid.New() | SecureRandom.uuid | Ramsey\\Uuid\\Uuid::uuid4() |
| Cryptographic pick | array[crypto.randomInt(0, array.length)] | secrets.choice(seq) | seq.get(sr.nextInt(seq.size())) | seq[crand.Int(...)] | seq[SecureRandom.random_number(seq.size)] | $seq[random_int(0, count($seq)-1)] |
A single canonical helper per codebase
1// lib/secure-random.ts — import this everywhere instead of reaching for
2// crypto / randomBytes / randomUUID at the call site. Makes grep trivial.
3
4import { randomBytes, randomInt, randomUUID } from 'node:crypto';
5
6/** URL-safe opaque token. Default 32 bytes = 256 bits. */
7export function secureToken(bytes = 32): string {
8 return randomBytes(bytes).toString('base64url');
9}
10
11/** 128-bit UUID v4 — fine for opaque IDs, NOT a secret. */
12export function secureId(): string {
13 return randomUUID();
14}
15
16/** Zero-padded numeric OTP. Unbiased. */
17export function secureOtp(digits = 6): string {
18 const ceil = 10 ** digits;
19 return randomInt(0, ceil).toString().padStart(digits, '0');
20}
21
22/** Uniform choice from a non-empty array. */
23export function secureChoice<T>(xs: readonly T[]): T {
24 if (xs.length === 0) throw new Error('empty array');
25 return xs[randomInt(0, xs.length)];
26}Make the safe path the short path
Teams that ship a single secureToken() / secureId() helper in a shared lib have dramatically fewer insecure-randomness findings than teams that let every call site reach for crypto primitives directly. It is a one-day project with a multi-year payoff. It also makes linting trivial: grep for Math.random and new Random and nothing else.
Your team adds a lint rule banning Math.random across the codebase. A developer requests an exception for `backoff = baseMs * (1 + Math.random())` in a retry library. Correct response?