Cryptographic Failures That Pass Code Review
Cryptographic code is uniquely dangerous, and it’s one of the areas I find most challenging to review. The reason is simple: it can be completely wrong and still appear to work perfectly. A broken hash function still produces a hash. A weak cipher still encrypts and decrypts. A predictable random number generator still generates numbers. The application runs, tests pass, and the vulnerability sits quietly until an attacker exploits it. In this post, I want to walk through the cryptographic failures that routinely survive code review across Python, Java, Go, and Rust, from the obvious use of MD5 to the subtle misuse of otherwise strong primitives.
Why Cryptographic Failures Are Different
Most vulnerability classes produce observable symptoms. SQL injection causes unexpected query results. XSS produces visible script execution. Buffer overflows crash the process. Cryptographic failures produce no symptoms at all during normal operation. The code does exactly what the developer intended, it just doesn’t provide the security guarantees the developer assumed. That’s what makes this category so insidious, and why crypto code deserves extra attention during reviews.
The three CWE categories covered here represent the most common failure modes:
- CWE-327: Use of a Broken or Risky Cryptographic Algorithm, Using algorithms with known weaknesses (DES, RC4, MD5 for integrity, SHA-1 for signatures).
- CWE-328: Use of Weak Hash, Using hash functions that are too fast for password storage (SHA-256 without key stretching) or that have known collision attacks (MD5, SHA-1).
- CWE-330: Use of Insufficiently Random Values, Using predictable random number generators for security-sensitive operations (tokens, keys, nonces).
The Easy-to-Spot Version
Python: MD5 for Password Hashing
import hashlib
def register_user(username, password):
password_hash = hashlib.md5(password.encode()).hexdigest()
db.execute(
"INSERT INTO users (username, password_hash) VALUES (?, ?)",
(username, password_hash)
)
return {"message": "User registered"}
MD5 is the canonical example of a broken hash. It has known collision attacks, it’s unsalted, and it’s fast enough to brute-force billions of hashes per second on consumer GPUs. Any reviewer who sees hashlib.md5 in a password context should flag it immediately. This still shows up in production code, though, usually in older systems that were written before password hashing best practices were widely understood.
Java: DES Encryption
import javax.crypto.Cipher;
import javax.crypto.spec.SecretKeySpec;
public byte[] encryptData(byte[] data, String key) throws Exception {
SecretKeySpec keySpec = new SecretKeySpec(key.getBytes("UTF-8"), "DES");
Cipher cipher = Cipher.getInstance("DES/ECB/PKCS5Padding");
cipher.init(Cipher.ENCRYPT_MODE, keySpec);
return cipher.doFinal(data);
}
DES uses a 56-bit key, which can be brute-forced in hours. ECB mode encrypts identical plaintext blocks to identical ciphertext blocks, leaking patterns. The "DES/ECB/PKCS5Padding" string is a red flag that any security-aware reviewer would catch. SAST tools like SpotBugs flag both DES and ECB mode with high confidence.
The Hard-to-Spot Version
These are the patterns that pass review because the developer chose algorithms that sound modern and secure, but used them incorrectly. These are the ones that genuinely worry me, and they’re worth studying closely.
Python: SHA-256 for Passwords (No Salt, No Stretching)
import hashlib
def hash_password(password):
return hashlib.sha256(password.encode()).hexdigest()
def verify_password(password, stored_hash):
return hashlib.sha256(password.encode()).hexdigest() == stored_hash
SHA-256 is not broken. It’s a perfectly good hash function for data integrity. But for password hashing, it has two critical problems: it’s unsalted (identical passwords produce identical hashes, enabling rainbow table attacks) and it’s fast (a modern GPU can compute billions of SHA-256 hashes per second).
Here’s what I find interesting about this one: a reviewer who knows “MD5 is bad” might see SHA-256 and assume it’s fine. The issue isn’t the algorithm, it’s the application. Password hashing requires a slow, salted, memory-hard function like bcrypt, scrypt, or Argon2. The idea that a “strong” hash can be wrong for a specific use case is counterintuitive, and it’s something I had to wrap my head around when I first started researching this topic.
Python: Predictable Random for Token Generation
import random
import string
def generate_session_token():
chars = string.ascii_letters + string.digits
return ''.join(random.choice(chars) for _ in range(32))
def generate_api_key():
return f"ak_{random.randint(100000000, 999999999)}"
Python’s random module uses the Mersenne Twister PRNG, which is not cryptographically secure. After observing 624 consecutive 32-bit outputs, an attacker can predict all future outputs. For session tokens and API keys, this means an attacker who can observe a few tokens can predict the next ones.
The fix is secrets.token_hex() or secrets.token_urlsafe(), which use the OS cryptographic random source. But random looks reasonable in review, it generates random-looking strings, and the 32-character length suggests the developer thought about entropy. This pattern can survive multiple rounds of code review because the output looks sufficiently random to the human eye. That’s the trap.
Java: AES with Static IV
import javax.crypto.Cipher;
import javax.crypto.spec.IvParameterSpec;
import javax.crypto.spec.SecretKeySpec;
public class DataEncryptor {
private static final byte[] FIXED_IV = "1234567890abcdef".getBytes();
private final SecretKeySpec keySpec;
public DataEncryptor(String key) {
this.keySpec = new SecretKeySpec(key.getBytes(), "AES");
}
public byte[] encrypt(byte[] data) throws Exception {
Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
cipher.init(Cipher.ENCRYPT_MODE, keySpec, new IvParameterSpec(FIXED_IV));
return cipher.doFinal(data);
}
public byte[] decrypt(byte[] data) throws Exception {
Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
cipher.init(Cipher.DECRYPT_MODE, keySpec, new IvParameterSpec(FIXED_IV));
return cipher.doFinal(data);
}
}
The developer chose AES (strong algorithm) with CBC mode (not ECB, good). But the initialization vector (IV) is a static constant. In CBC mode, a static IV means that encrypting the same plaintext with the same key always produces the same ciphertext, which leaks information about whether two plaintexts are identical. Worse, it enables certain chosen-plaintext attacks.
A reviewer who checks for “is this AES?” and “is this not ECB?” might pass this code. The IV issue requires deeper cryptographic knowledge to spot. The fix is generating a random IV for each encryption operation and prepending it to the ciphertext. This is one of those bugs where the developer did 90% of the work right, they just missed the last critical detail.
Java: SecureRandom with Predictable Seed
import java.security.SecureRandom;
public class TokenGenerator {
private final SecureRandom random;
public TokenGenerator() {
this.random = new SecureRandom();
this.random.setSeed(System.currentTimeMillis());
}
public String generateToken() {
byte[] bytes = new byte[32];
random.nextBytes(bytes);
StringBuilder sb = new StringBuilder();
for (byte b : bytes) {
sb.append(String.format("%02x", b));
}
return sb.toString();
}
}
SecureRandom is Java’s cryptographically secure PRNG, the right choice. But calling setSeed(System.currentTimeMillis()) replaces the OS-provided entropy with a predictable value. The system time at startup is guessable to within a few seconds, reducing the effective entropy from 256 bits to roughly 10-15 bits. An attacker who knows approximately when the server started can enumerate all possible seeds.
The fix is to simply not call setSeed() at all. SecureRandom seeds itself from the OS entropy source by default. The explicit seeding was likely added by a developer who thought they were “helping” the randomness. This is a great example of how crypto is one of those areas where doing less is often more secure, well-intentioned additions can actually make things worse.
Go: Weak Random for Cryptographic Purposes
import (
"fmt"
"math/rand"
"time"
)
func generateResetToken() string {
rand.Seed(time.Now().UnixNano())
const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
token := make([]byte, 32)
for i := range token {
token[i] = charset[rand.Intn(len(charset))]
}
return string(token)
}
Go has two random packages: math/rand (fast, predictable) and crypto/rand (cryptographically secure). The math/rand package seeded with time.Now().UnixNano() is predictable, the seed has nanosecond resolution, but an attacker who knows the approximate time of token generation can brute-force the seed space.
This is a common Go mistake, because math/rand is the first result when searching for “Go random” and its API is more convenient than crypto/rand. The fix:
import "crypto/rand"
func generateResetToken() string {
bytes := make([]byte, 32)
if _, err := rand.Read(bytes); err != nil {
panic("crypto/rand failed")
}
return hex.EncodeToString(bytes)
}
Go: Insecure TLS Configuration
import "crypto/tls"
func createHTTPClient() *http.Client {
tlsConfig := &tls.Config{
InsecureSkipVerify: true,
}
transport := &http.Transport{
TLSClientConfig: tlsConfig,
}
return &http.Client{Transport: transport}
}
InsecureSkipVerify: true disables TLS certificate verification, making the connection vulnerable to man-in-the-middle attacks. This is often added during development to work with self-signed certificates and never removed. The code compiles cleanly, the HTTP client works, and the connection is still encrypted, it just doesn’t verify who it’s talking to. This shows up in production Go code surprisingly often. It’s one of those “temporary” fixes that becomes permanent because everything appears to work fine.
Rust: Using rand Instead of rand with OsRng
use rand::Rng;
fn generate_api_token() -> String {
let mut rng = rand::thread_rng();
let token: String = (0..32)
.map(|_| {
let idx = rng.gen_range(0..62);
let chars = b"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
chars[idx] as char
})
.collect();
token
}
In Rust, rand::thread_rng() actually uses a cryptographically secure generator (ChaCha) seeded from the OS. So this code is technically safe in current versions of the rand crate. The subtle issue arises when developers use rand::rngs::SmallRng or rand::rngs::StdRng::seed_from_u64() for “performance” in security contexts:
use rand::rngs::SmallRng;
use rand::SeedableRng;
fn generate_token_fast() -> String {
let mut rng = SmallRng::seed_from_u64(
std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap()
.as_nanos() as u64
);
// ... generate token using rng
}
SmallRng is explicitly documented as not cryptographically secure. Seeding it from the system time makes it predictable. A reviewer who sees rand in Rust might assume it’s safe without checking which specific RNG type is being used. What I find interesting here is that Rust developers tend to trust the type system to catch these issues, but the type system doesn’t know the difference between “random for a game” and “random for a security token”, that’s a semantic distinction only a human reviewer can make.
Rust: Hardcoded Encryption Key
use aes_gcm::{Aes256Gcm, Key, Nonce};
use aes_gcm::aead::Aead;
use aes_gcm::KeyInit;
const ENCRYPTION_KEY: &[u8; 32] = b"my-super-secret-key-1234567890!!";
fn encrypt_sensitive_data(plaintext: &[u8]) -> Vec<u8> {
let key = Key::<Aes256Gcm>::from_slice(ENCRYPTION_KEY);
let cipher = Aes256Gcm::new(key);
let nonce = Nonce::from_slice(b"unique nonce");
cipher.encrypt(nonce, plaintext).expect("encryption failed")
}
The developer chose AES-256-GCM, an excellent authenticated encryption algorithm. But the key is a hardcoded constant compiled into the binary, and the nonce is static. Anyone with access to the binary can extract the key. The static nonce means encrypting the same plaintext twice produces identical ciphertext, and reusing a nonce with GCM completely breaks its authentication guarantees. Something worth remembering: GCM with a reused nonce is worse than no encryption at all, because it gives you a false sense of security while being trivially breakable.
Detection Strategies
SAST Tool Coverage
| Language | Tool | Catches Easy | Catches Hard |
|---|---|---|---|
| Python | Bandit | ✓ (MD5, DES, random) |
Partial (SHA-256 for passwords) |
| Java | SpotBugs | ✓ (DES, ECB) | Partial (static IV, seeded SecureRandom) |
| Go | gosec | ✓ (math/rand, InsecureSkipVerify) | Limited (TLS config in helpers) |
| Rust | cargo-audit | ✓ (vulnerable crate versions) | No (logic-level crypto misuse) |
The hard-to-spot versions evade SAST, and here’s what I think explains it:
- The algorithm itself is strong (AES-256, SHA-256), the misuse is in how it’s applied
- Static IVs and predictable seeds require understanding cryptographic requirements, not just pattern matching
- Password hashing with SHA-256 looks identical to legitimate data integrity hashing
This is why SAST alone isn’t sufficient for crypto review. You need someone who understands the cryptographic requirements of each use case.
Manual Review Checklist
Here’s a checklist that works well for reviewing crypto code:
- Password storage: Verify bcrypt, scrypt, or Argon2 is used, not any general-purpose hash function
- Random number generation: Trace every random call to its source.
math/rand,random.Random,java.util.Randomare not cryptographically secure - Encryption IVs/nonces: Verify they are generated randomly per operation, not static constants
- Key management: Search for hardcoded keys, keys derived from predictable values, and keys stored in source code
- TLS configuration: Search for
InsecureSkipVerify,verify=False, disabled certificate checks - Algorithm selection: Flag DES, 3DES, RC4, MD5 (for security), SHA-1 (for signatures). Verify AES uses CBC or GCM, not ECB
Remediation
Password Hashing, Use Purpose-Built Functions
# Python
import bcrypt
def hash_password(password: str) -> str:
return bcrypt.hashpw(password.encode(), bcrypt.gensalt()).decode()
def verify_password(password: str, stored_hash: str) -> bool:
return bcrypt.checkpw(password.encode(), stored_hash.encode())
// Java
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
BCryptPasswordEncoder encoder = new BCryptPasswordEncoder(12);
String hash = encoder.encode(password);
boolean matches = encoder.matches(password, hash);
// Go
import "golang.org/x/crypto/bcrypt"
func hashPassword(password string) (string, error) {
bytes, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost)
return string(bytes), err
}
Cryptographically Secure Random
# Python
import secrets
token = secrets.token_urlsafe(32)
// Go
import "crypto/rand"
bytes := make([]byte, 32)
rand.Read(bytes)
// Rust
use rand::rngs::OsRng;
use rand::RngCore;
let mut key = [0u8; 32];
OsRng.fill_bytes(&mut key);
AES with Random IV
// Java
import javax.crypto.Cipher;
import javax.crypto.spec.GCMParameterSpec;
import java.security.SecureRandom;
public byte[] encrypt(byte[] data, SecretKeySpec keySpec) throws Exception {
byte[] iv = new byte[12]; // 96-bit IV for GCM
new SecureRandom().nextBytes(iv);
Cipher cipher = Cipher.getInstance("AES/GCM/NoPadding");
cipher.init(Cipher.ENCRYPT_MODE, keySpec, new GCMParameterSpec(128, iv));
byte[] ciphertext = cipher.doFinal(data);
// Prepend IV to ciphertext for decryption
byte[] result = new byte[iv.length + ciphertext.length];
System.arraycopy(iv, 0, result, 0, iv.length);
System.arraycopy(ciphertext, 0, result, iv.length, ciphertext.length);
return result;
}
The core principle across all languages: use high-level cryptographic libraries that make the secure path the default path. Don’t reach for low-level primitives unless you have a specific reason and the expertise to use them correctly. When in doubt, use bcrypt for passwords, AES-256-GCM for encryption with random nonces, and your language’s cryptographic random source for all security-sensitive randomness. Following these simple rules eliminates the vast majority of cryptographic failures before they ever make it to production.