Security Review

Security Review

Systematic code-level security analysis: OWASP Top 10, static analysis, variant analysis, and cryptographic review.

Core principle: Every finding requires a concrete exploit scenario. Pattern matches without proven impact are not findings.

When to Use

• Auditing code for specific vulnerability classes
• Running Semgrep/CodeQL static analysis
• Reviewing auth, session, or input handling code
• After finding one vulnerability (variant analysis)
• Reviewing cryptographic code for timing side-channels

When NOT to Use

• System-level threat modeling (use threat-modeling skill)
• Dependency vulnerability scanning (use security-hardening skill)
• Compliance gap analysis (use compliance-and-governance skill)

Phase 1: Static Analysis

Semgrep

semgrep scan \
  --config p/security-audit \
  --config p/owasp-top-ten \
  --severity HIGH --severity CRITICAL \
  --metrics=off \
  --json -o results.json .

Always use --metrics=off during audits.

Key rulesets by language:

For injection (SQLi, SSRF, path traversal): use taint-mode rules — they track data from source to sink through function calls.

Test-driven rule creation: Write test cases BEFORE writing the rule pattern.

1. Define true positives (code that should match) and true negatives (safe code that must not match)
2. Run semgrep --test to verify: iterate on the AST pattern until all tests pass
3. For data-flow vulnerabilities, use mode: taint with explicit sources and sinks
4. Only finalize the rule after the test suite is green — pattern-first rules have high FP rates

CodeQL (Deep Analysis)

codeql database create codeql.db --language=javascript
codeql database analyze codeql.db \
  javascript-security-extended.qls \
  --format=sarif-latest -o results.sarif

Zero findings needs investigation, not celebration. Zero results can indicate poor database quality or missing data flow models.

ASVS 5.0 Verification Levels

Start every audit at L1. Escalate to L2/L3 based on data sensitivity and threat model. L3 requires formal cryptographic review.

Phase 2: OWASP Top 10:2025

Phase 3: Input Validation Checklist

[ ] All user input validated server-side (never trust client-side only)
[ ] Parameterized queries for all DB access (never string concatenation)
[ ] Input length limits enforced
[ ] Allowlist validation preferred over denylist
[ ] File uploads: type, size, magic bytes checked; stored outside webroot
[ ] HTML output: escaped or sanitized before rendering
[ ] No eval()/exec() on user input
[ ] Template injection impossible (no user-controlled template strings)

Phase 4: Authentication and Session Review

[ ] Passwords: Argon2id or bcrypt (rounds >= 12), never MD5/SHA1
[ ] Session tokens: 128+ bits entropy, httpOnly, Secure, SameSite=Strict
[ ] Sessions invalidated on logout (server-side invalidation)
[ ] MFA available for sensitive operations
[ ] Rate limiting on /login, /register, /forgot-password
[ ] Access tokens: short-lived (15 min); refresh tokens: rotated on use
[ ] No credentials in URLs, query params, or logs
[ ] Password reset tokens: single-use, expire in 1 hour

Phase 5: Insecure Defaults Detection

Find fail-open vulnerabilities — where missing config makes the app run insecurely:

# FAIL-OPEN (CRITICAL): App runs with weak secret when env var missing
SECRET_KEY = os.environ.get('SECRET_KEY') or 'default-secret'

# FAIL-SECURE (SAFE): App crashes if secret missing
SECRET_KEY = os.environ['SECRET_KEY']

Detection patterns (grep these):

# Fallback secrets
grep -r "getenv.*) or ['\"]" src/
grep -r "ENV.*|| ['\"]" src/

# Hardcoded credentials
grep -r "password.*=.*['\"][^'\"]{8,}['\"]" src/
grep -r "secret.*=.*['\"][^'\"]{8,}['\"]" src/

# Weak defaults
grep -r "DEBUG.*=.*true" src/
grep -r "verify.*=.*false" src/

Phase 6: Sharp Edges Analysis

Pit of Success — secure usage must be the path of least resistance. Evaluate through three adversary lenses:

• Scoundrel: Can a malicious developer disable security via configuration?
• Lazy Developer: Does copy-pasting the first example produce insecure code?
• Confused Developer: Can parameters be swapped without type errors, producing silent misuse?

Edge case probing: For every security-relevant parameter, ask: What happens with 0, "", null, [], -1? Is the default the most secure option?

Phase 7: Constant-Time Analysis

For code handling secrets, tokens, or passwords:

// Safe string comparison (Node.js)
import { timingSafeEqual } from 'crypto';

function safeCompare(a: string, b: string): boolean {
  const bufA = Buffer.from(a);
  const bufB = Buffer.from(b);
  if (bufA.length !== bufB.length) return false;
  return timingSafeEqual(bufA, bufB);
}

Variant Analysis

When you find one vulnerability, systematically search for similar instances using this 5-step workflow:

1. Understand root cause — identify the exact condition and data flow, not just the symptom
2. Create an exact match — write a pattern that matches only the known instance; confirm it fires
3. Identify abstraction points — what can be generalized? (variable name, function, call site, module)
4. Generalize one element at a time — change one thing per iteration; re-run and review new matches
5. Triage — stop when false positive rate exceeds 50%; document and prioritize real findings

Search the ENTIRE codebase, not just the module where the original was found. One confirmed SQL injection often has 5+ variants across different query paths. Use Semgrep for fast pattern matching; escalate to CodeQL for interprocedural data flow.

False Positive Filtering

Never report:

• Test fixtures, example files, documentation examples
• Dependency CVEs with CVSS <4.0 and no known exploit
• Secrets committed and removed in same initial-setup PR

Verification requirements before reporting:

• Secrets: Validate key format. Do NOT test against live APIs.
• Injection: Trace from user input to dangerous sink. Confirm no sanitization in path.
• Auth bypass: Prove the bypass works with a concrete example.

Confidence scoring:

Red Flags (Immediate Investigation)

• User input passed directly to SQL, shell, or HTML
• Secrets in source code or commit history
• API endpoints without authentication or authorization
• Wildcard CORS (*) in production
• No rate limiting on auth endpoints
• Stack traces exposed to users
• eval()/exec() processing user or LLM output
• Webhook endpoints without signature verification
• Fail-open error handlers: catch { return true }
• ${{ github.event.* }} in CI/CD prompt fields

Verification Checklist

• Static analysis run (Semgrep + language-appropriate rulesets)
• All A01-A10 OWASP items checked
• Input validation at all user-facing boundaries
• Auth and session checklist completed
• Insecure defaults scan performed
• Variant analysis done for any confirmed finding
• Constant-time comparisons used for all secrets
• Every finding verified with concrete exploit scenario
• False positives filtered before reporting