Security Review

Security Review

Complete framework for conducting security code reviews and identifying vulnerabilities before they reach production.

When to Use

• Reviewing code before merging to production
• Auditing existing codebase for vulnerabilities
• Assessing third-party dependencies
• Preparing for security audits
• Training developers on secure coding
• Investigating security incidents

OWASP Top 10 Security Risks

Critical vulnerabilities to check for:

1. Injection (SQL, Command, LDAP)
2. Broken Authentication
3. Sensitive Data Exposure
4. XML External Entities (XXE)
5. Broken Access Control
6. Security Misconfiguration
7. Cross-Site Scripting (XSS)
8. Insecure Deserialization
9. Vulnerable Components
10. Insufficient Logging & Monitoring

Workflow

Step 1: Risk Assessment

Determine the security criticality:

## Security Risk Assessment: [Feature/PR]

**Data Sensitivity**:
- [ ] None (public data only)
- [ ] Low (non-sensitive user data)
- [ ] Medium (PII, email, phone)
- [ ] High (passwords, payment info)
- [ ] Critical (health records, financial data)

**Attack Surface**:
- [ ] Internal only (admin tools)
- [ ] Authenticated users
- [ ] Public (unauthenticated)

**Impact if Compromised**:
- [ ] Low (minor inconvenience)
- [ ] Medium (data leak, service disruption)
- [ ] High (financial loss, reputation damage)
- [ ] Critical (legal liability, major breach)

**Overall Risk Level**: [Low / Medium / High / Critical]

Step 2: Input Validation Review

Check all user input is validated:

## Input Validation Checklist

**Server-Side Validation**:
- [ ] All user input validated on server (never trust client)
- [ ] Validation uses allowlists, not blocklists
- [ ] Input length limits enforced
- [ ] Special characters properly handled
- [ ] File uploads validated (type, size, name)
- [ ] URL parameters sanitized
- [ ] JSON/XML parsed safely

**Specific Checks**:
- [ ] Email format validation
- [ ] Phone number format
- [ ] Date/time format
- [ ] Numeric ranges
- [ ] Enum values

Example: Safe Input Validation

// ❌ VULNERABLE: Trusts client-side validation
app.post('/api/users', (req, res) => {
  const { email } = req.body;
  db.query(`INSERT INTO users (email) VALUES ('${email}')`);
});

// ✅ SECURE: Server-side validation + parameterized queries
const { z } = require('zod');

const userSchema = z.object({
  email: z.string().email().max(255),
  age: z.number().int().min(13).max(120)
});

app.post('/api/users', (req, res) => {
  // Validate input
  const result = userSchema.safeParse(req.body);
  if (!result.success) {
    return res.status(400).json({ error: result.error });
  }

  // Use parameterized query
  const { email, age } = result.data;
  db.query('INSERT INTO users (email, age) VALUES ($1, $2)', [email, age]);
});

Step 3: Injection Prevention

SQL Injection:

// ❌ VULNERABLE
const query = `SELECT * FROM users WHERE email = '${email}'`;
db.query(query);

// ✅ SECURE: Parameterized queries
const query = 'SELECT * FROM users WHERE email = $1';
db.query(query, [email]);

// ✅ SECURE: ORM with prepared statements
const user = await User.findOne({ where: { email } });

Command Injection:

// ❌ VULNERABLE
const { exec } = require('child_process');
exec(`convert ${filename} output.png`);

// ✅ SECURE: Use execFile (doesn't spawn shell)
const { execFile } = require('child_process');
execFile('convert', [filename, 'output.png']);

// ✅ BETTER: Use library that doesn't execute commands
const sharp = require('sharp');
await sharp(filename).toFile('output.png');

Path Traversal:

// ❌ VULNERABLE
const filePath = `/uploads/${userInput}`;
fs.readFile(filePath);

// ✅ SECURE
const path = require('path');
const safeName = path.basename(userInput);
const filePath = path.join('/uploads', safeName);

// Verify it's within expected directory
const realPath = path.resolve(filePath);
if (!realPath.startsWith(path.resolve('/uploads'))) {
  throw new Error('Invalid path');
}

Step 4: Authentication Review

## Authentication Checklist

**Password Security**:
- [ ] Passwords hashed with bcrypt/argon2 (NOT md5/sha1)
- [ ] Minimum password requirements enforced
- [ ] No plain text passwords anywhere (code, logs, DB)
- [ ] Password reset is secure (token-based, time-limited)

**Session Management**:
- [ ] Session tokens are random and long (128+ bits)
- [ ] Sessions stored server-side
- [ ] Session invalidated on logout
- [ ] Session timeout implemented (30 min inactivity)
- [ ] Tokens use httpOnly and secure flags

**Brute Force Protection**:
- [ ] Rate limiting on login attempts
- [ ] Account lockout after X failed attempts
- [ ] CAPTCHA after multiple failures

**Multi-Factor Authentication**:
- [ ] MFA option available (TOTP, SMS, hardware key)
- [ ] Backup codes provided

Example: Secure Password Hashing

const bcrypt = require('bcrypt');

// ❌ VULNERABLE
function hashPassword(password) {
  return crypto.createHash('md5').update(password).digest('hex');
}

// ✅ SECURE
async function hashPassword(password) {
  const saltRounds = 12;
  return await bcrypt.hash(password, saltRounds);
}

async function verifyPassword(password, hash) {
  return await bcrypt.compare(password, hash);
}

Step 5: Authorization Review

## Authorization Checklist

**Access Control**:
- [ ] Authorization checked on EVERY request
- [ ] Authorization checked server-side (never client)
- [ ] Principle of least privilege applied
- [ ] No direct object references without auth check
- [ ] Admin functions properly protected
- [ ] Row-level security where needed (multi-tenant)

**Common Patterns**:
- [ ] Check user owns resource before modification
- [ ] Verify user role/permissions
- [ ] Prevent privilege escalation

Example: Insecure Direct Object Reference (IDOR)

// ❌ VULNERABLE: Anyone can access any order
app.get('/api/orders/:id', (req, res) => {
  const order = db.getOrder(req.params.id);
  res.json(order);
});

// ✅ SECURE: Verify user owns the order
app.get('/api/orders/:id', requireAuth, (req, res) => {
  const order = db.getOrder(req.params.id);

  // Authorization check
  if (order.userId !== req.user.id) {
    return res.status(403).json({ error: 'Forbidden' });
  }

  res.json(order);
});

Step 6: XSS Prevention

Cross-Site Scripting (XSS) Types:

Reflected XSS:

// ❌ VULNERABLE
app.get('/search', (req, res) => {
  const query = req.query.q;
  res.send(`<h1>Results for: ${query}</h1>`);
});

// ✅ SECURE: Escape output
const escapeHtml = require('escape-html');
app.get('/search', (req, res) => {
  const query = escapeHtml(req.query.q);
  res.send(`<h1>Results for: ${query}</h1>`);
});

Stored XSS:

// ❌ VULNERABLE: Storing unsan itized HTML
element.innerHTML = userInput;

// ✅ SECURE: Use textContent for text
element.textContent = userInput;

// ✅ SECURE: If HTML needed, sanitize first
import DOMPurify from 'dompurify';
element.innerHTML = DOMPurify.sanitize(userInput);

DOM-based XSS:

// ❌ VULNERABLE
const name = location.hash.substring(1);
document.write('Hello ' + name);

// ✅ SECURE
const name = location.hash.substring(1);
const safe = document.createTextNode('Hello ' + name);
document.body.appendChild(safe);

Step 7: Data Protection

## Data Protection Checklist

**Encryption**:
- [ ] Sensitive data encrypted at rest
- [ ] TLS/HTTPS used for data in transit
- [ ] Encryption keys not in code
- [ ] Strong algorithms (AES-256, not DES)

**Secrets Management**:
- [ ] Secrets not in code or version control
- [ ] Secrets not in logs or error messages
- [ ] Environment variables or secrets manager used
- [ ] API keys rotated regularly

**PII Handling**:
- [ ] PII handled according to privacy policy
- [ ] Data retention policies followed
- [ ] Secure deletion when required
- [ ] Data minimization (collect only what's needed)

Example: Secrets Management

// ❌ VULNERABLE
const API_KEY = 'sk_live_abc123...';  // Hardcoded!

// ✅ SECURE
const API_KEY = process.env.STRIPE_API_KEY;

// Validate secret is loaded
if (!API_KEY) {
  throw new Error('STRIPE_API_KEY environment variable not set');
}

Step 8: Security Headers

Essential Security Headers:

app.use((req, res, next) => {
  // Prevent MIME type sniffing
  res.setHeader('X-Content-Type-Options', 'nosniff');

  // Enable XSS filter
  res.setHeader('X-XSS-Protection', '1; mode=block');

  // Prevent clickjacking
  res.setHeader('X-Frame-Options', 'DENY');

  // Control referrer information
  res.setHeader('Referrer-Policy', 'strict-origin-when-cross-origin');

  // Content Security Policy (adjust for your app)
  res.setHeader(
    'Content-Security-Policy',
    "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'"
  );

  // HSTS (force HTTPS)
  res.setHeader(
    'Strict-Transport-Security',
    'max-age=31536000; includeSubDomains'
  );

  next();
});

Step 9: Dependency Security

## Dependency Security Checklist

**Vulnerability Scanning**:
- [ ] Run `npm audit` or `yarn audit` regularly
- [ ] Automated scanning in CI/CD
- [ ] Update dependencies monthly
- [ ] Pin dependency versions (avoid `^` or `~`)

**Supply Chain**:
- [ ] Review new dependencies before adding
- [ ] Check dependency maintainership
- [ ] Avoid packages with no recent updates
- [ ] Use lock files (package-lock.json, yarn.lock)

Check for Vulnerabilities:

# NPM
npm audit
npm audit fix

# Yarn
yarn audit
yarn upgrade-interactive --latest

# Python
pip-audit

# Ruby
bundler-audit

Step 10: Logging & Monitoring

## Logging & Monitoring Checklist

**What to Log**:
- [ ] Authentication events (login, logout, failed)
- [ ] Authorization failures
- [ ] Input validation failures
- [ ] Security-relevant actions (password change, role change)
- [ ] Errors and exceptions

**What NOT to Log**:
- [ ] Passwords (NEVER)
- [ ] Session tokens
- [ ] Full credit card numbers
- [ ] PII (unless required and encrypted)
- [ ] API keys/secrets

Example: Safe Logging

// ❌ BAD: Logs password
logger.info({ user: email, password }, 'Login attempt');

// ✅ GOOD: No sensitive data
logger.info({ user: email, success: true }, 'Login successful');

// ❌ BAD: Logs full error with stack trace to user
catch (error) {
  res.status(500).json({ error: error.message, stack: error.stack });
}

// ✅ GOOD: Generic error to user, detailed logs server-side
catch (error) {
  logger.error({ error, userId, action }, 'Operation failed');
  res.status(500).json({ error: 'An error occurred' });
}

Security Review Template

## Security Review: [Feature/PR]

**Reviewer**: [Name]
**Date**: [Date]
**Code Reviewed**: [Files/PR Link]

### Risk Assessment
- **Data Sensitivity**: [None/Low/Medium/High/Critical]
- **Attack Surface**: [Internal/Authenticated/Public]
- **Overall Risk**: [Low/Medium/High/Critical]

### Findings

#### Critical (Fix Before Merge)
| ID | Issue | Location | Remediation |
|----|-------|----------|-------------|
| C1 | SQL Injection | users.js:45 | Use parameterized queries |

#### High (Fix Soon)
| ID | Issue | Location | Remediation |
|----|-------|----------|-------------|
| H1 | Missing authorization | orders.js:22 | Add ownership check |

#### Medium (Track for Future)
| ID | Issue | Location | Remediation |
|----|-------|----------|-------------|
| M1 | Weak session timeout | auth.js:10 | Reduce from 24h to 30min |

#### Low/Informational
| ID | Issue | Location | Remediation |
|----|-------|----------|-------------|
| L1 | Consider rate limiting | api.js:5 | Add per-user rate limits |

### Checklist Results
| Category | Status | Notes |
|----------|--------|-------|
| Input Validation | ✅ | All inputs validated |
| Authentication | ❌ | No MFA option |
| Authorization | ✅ | Ownership checks in place |
| Data Protection | ⚠️ | Secrets in env vars but not rotated |
| XSS Prevention | ✅ | Outputs properly escaped |
| Injection Prevention | ❌ | SQL injection found |
| Security Headers | ✅ | All headers configured |
| Dependencies | ⚠️ | 2 medium vulnerabilities |
| Logging | ✅ | No sensitive data logged |

### Recommendations
1. **Critical**: Fix SQL injection in users.js before merge
2. **High**: Add MFA option in next sprint
3. **Medium**: Set up automated dependency scanning

### Sign-off
- [ ] All critical findings addressed
- [ ] All high findings addressed or have tracking tickets
- [ ] Medium findings tracked for follow-up

Common Vulnerabilities & Fixes

Tools & Resources

Automated Scanning:

• Snyk (dependency scanning)
• OWASP ZAP (web app scanner)
• Bandit (Python security linter)
• Brakeman (Ruby on Rails scanner)

Manual Review:

• OWASP Top 10 Checklist
• Security Code Review Guide
• CWE/SANS Top 25

Training:

• OWASP WebGoat (hands-on practice)
• PentesterLab (security challenges)
• HackerOne CTF

Related Skills

• /code-review - General code quality review
• /performance-optimization - Performance considerations
• /devops-cicd - Security in CI/CD pipelines

Last Updated: 2026-01-22