Security Review Guide
Comprehensive vulnerability taxonomy and review methodology for code security reviews. Project-specific security policies (CLAUDE.md, security docs) always take priority over this guidance.
Common Vulnerability Classes
Injection
- SQL injection: Unsanitized user input in queries; use parameterized queries or ORM query builders
- Command injection: User input passed to shell commands; avoid shell execution or use strict allowlists
- LDAP injection: Unsanitized input in LDAP queries
- Header injection: User input in HTTP headers enabling response splitting
Cross-Site Scripting (XSS)
- Reflected: User input echoed in responses without encoding
- Stored: Persistent user input rendered without encoding
- DOM-based: Client-side JavaScript manipulating DOM with unsanitized data
Cross-Site Request Forgery (CSRF)
- Missing or weak CSRF token validation
- Token not bound to user session
- GET requests performing state changes
Authentication Flaws
- Weak credential handling or storage
- Insecure session management
- Missing authentication checks on protected endpoints
Authorization Flaws
- Broken access control, privilege escalation
- Insecure Direct Object References (IDOR)
- Missing tenant isolation in multi-tenant systems
Cryptographic Issues
- Weak algorithms (MD5, SHA1 for security purposes)
- Improper key management or hardcoded secrets
- Insufficient entropy in token/nonce generation
Information Disclosure
- Sensitive data in logs, error messages, or API responses
- Stack traces or debug info exposed in production
- Credentials or tokens in URLs
Insecure Deserialization
- Unsafe parsing of untrusted data formats
- Object instantiation from user-controlled input
Domain-Specific Concerns
Race Conditions
- TOCTOU bugs: Time-of-check-to-time-of-use allowing business logic violations (double-spending, duplicate issuance)
- Concurrent access: Missing locking or optimistic concurrency control on shared resources
OAuth 2.0 Pitfalls
- Token leakage via referrer headers or logs
- Insufficient scope validation
- Redirect URI manipulation (open redirect, partial matching)
- Authorization code replay
- PKCE bypass or missing PKCE enforcement
- Refresh token rotation issues
OpenID Connect Pitfalls
- ID token validation gaps (signature, expiry, issuer, audience)
- Nonce misuse or missing nonce validation
at_hash / c_hash verification omitted- Issuer confusion in multi-provider setups
Session Security
- Session fixation or hijacking
- Insufficient expiration (idle and absolute timeouts)
- Cookie attribute misconfiguration (missing Secure, HttpOnly, SameSite)
Timing Attacks
- Non-constant-time comparisons on secrets, tokens, or hashes — all secret comparisons (passwords, tokens, HMAC digests, API keys) must use constant-time operations (e.g.,
ActiveSupport::SecurityUtils.secure_compare, hmac.compare_digest, crypto.timingSafeEqual)
- Observable timing differences in authentication flows (e.g., different response times for valid vs invalid usernames)
- Database lookups that short-circuit on missing records before performing credential verification
Review Methodology
- Read the actual code — do not rely on summaries or assumptions; open and read every file under review and follow references to understand the full call chain
- Analyze each change against the vulnerability classes above
- Check surrounding code — vulnerabilities often arise from interactions between new and existing code
- Verify both presence and absence — confirm mitigations exist where needed, and note areas reviewed that appeared secure
Severity Classification
- Critical: Remotely exploitable, no authentication required, leads to full system compromise or mass data breach
- High: Exploitable with low complexity, leads to significant data exposure or privilege escalation
- Medium: Requires specific conditions to exploit, limited blast radius
- Low: Minor issue, difficult to exploit, minimal impact
- Informational: Best practice deviation, defense-in-depth recommendation