analyzing-code-security

What this skill does

## Security Review Workflow

Follow these steps when conducting a manual security code review:

1. **Identify the attack surface.** Determine entry points: API endpoints, message handlers, file parsers, user-facing forms. Read route definitions and controller registrations to build a map.
2. **Trace data flows from sources to sinks.** Follow untrusted input (HTTP parameters, headers, request bodies, file uploads, external API responses) through all transformations to dangerous operations (database queries, command execution, HTML rendering, file system access).
3. **Check trust boundary crossings.** At every point where data crosses a trust boundary (client→server, service→service, user input→database), verify that validation, authentication, and authorization are enforced.
4. **Apply framework checklists.** Consult `references/framework-checklists.md` for OWASP Web/API/Mobile Top 10 and CWE Top 25. Check each applicable category against the code under review.
5. **Adopt an adversarial mindset.** Form a hypothesis (e.g., "I can bypass SSO", "I can access another user's vault") and work backwards to determine what conditions would make it exploitable.
6. **Map findings to CWE IDs.** Every finding must include the specific CWE identifier, the code location, and the data flow that makes it exploitable.
7. **Classify by practical exploitability.** Distinguish between practically exploitable vulnerabilities and theoretical risks. Prioritize accordingly but document both.

## Key Vulnerability Categories

The most frequently encountered categories across Bitwarden's stack:

- **Injection** (CWE-89, CWE-78, CWE-77) — Unsanitized input reaching SQL queries, OS commands, or LDAP queries. Always use parameterized queries and avoid string concatenation.
- **Broken Access Control** (CWE-862, CWE-287, CWE-306) — Missing authorization checks, IDOR, privilege escalation. Verify per-object ownership checks and role enforcement at every layer.
- **XSS** (CWE-79) — User input rendered in HTML without encoding. In Angular, avoid `innerHTML` and `bypassSecurityTrust*` with untrusted content.
- **SSRF** (CWE-918) — User-controlled URLs in server-side requests. Validate against host allowlists.
- **Insecure Deserialization** (CWE-502) — Type-handling enabled on untrusted input. Avoid `TypeNameHandling.All` in JSON.NET.
- **Path Traversal** (CWE-22) — User-supplied paths reaching file system operations. Canonicalize and validate against a base directory.
- **Cryptographic Failures** — Weak algorithms, hardcoded keys, predictable IVs. See the `reviewing-security-architecture` skill for approved algorithms.

For complete framework checklists (all OWASP and CWE categories), consult **`references/framework-checklists.md`**.

For CORRECT/WRONG code examples in C#, TypeScript, and SQL, consult **`references/vulnerability-patterns.md`**.

## Adversarial Review Mindset

Adopt an adversarial mindset during security code review — this differs from regular code review which seeks to strengthen code.

**How to think adversarially:**

1. **Create a hypothesis** — e.g., "I can bypass SSO", "I can access another user's vault", "I can escalate from member to admin"
2. **Work backwards** — What conditions would need to be true for the attack to succeed? Can those conditions be fabricated?
3. **Question assumptions** — Is that authorization check always reached? What happens if the middleware fails? What if the token is malformed but not invalid?
4. **Consider failure modes** — What happens when things fail? Do they fail open (insecure) or fail closed (secure)?

## Critical Rules

- **Authentication before authorization.** Always verify the user is who they claim to be before checking what they're allowed to do. Never skip auth checks in "internal" endpoints.
- **Validate at trust boundaries.** Every point where data crosses a trust boundary (client→server, service→service, user input→database) must validate. Never trust client-side validation alone.
- **Map findings to CWE IDs.** Every finding must include a specific CWE identifier with evidence: the code location and the data flow that makes it exploitable.
- **Practical over theoretical.** Distinguish between vulnerabilities that are practically exploitable in this system vs. theoretical risks. Prioritize accordingly but document both.
- **Check the whole chain.** A vulnerability isn't just the sink — trace from the source (user input) through all transformations to the sink (dangerous operation). If the chain is broken by sanitization, it's not exploitable.

## Additional Resources

### Reference Files

For detailed checklists and code examples, consult:

- **`references/framework-checklists.md`** — OWASP Web Top 10, API Top 10, Mobile Top 10 (2024), CWE Top 25 lookup tables
- **`references/vulnerability-patterns.md`** — CORRECT/WRONG code examples for C#/.NET, TypeScript/Angular, and SQL