Security Review Skill

Perform a security-focused code review that identifies real, exploitable vulnerabilities and provides proof-of-concept demonstrations. Produces a high-signal report with two distinct sections: confirmed vulnerabilities with PoCs, and separate security hardening recommendations.

Critical Principles

1. Only report real vulnerabilities. Every vulnerability in the report must be grounded in how the code actually works, not how it theoretically could be misused. Trace data flow from attacker-controlled input to the dangerous operation. If the path is blocked by validation, type constraints, or other controls, it is not a vulnerability.

2. Investigate before reporting. If you encounter a pattern that looks suspicious but you are not certain it is exploitable, DO NOT include it in the report as a vulnerability. Instead, investigate it:

   • Use the Task tool with

     subagent_type: "general-purpose"

     to spawn an investigation agent. Instruct it to trace the data flow, read all relevant code paths, and determine whether the issue is actually exploitable.
   • Only include the finding if the investigation confirms it is a real problem.

3. Every vulnerability needs a proof of concept. If you cannot write a PoC that demonstrates the vulnerability, it does not belong in the Vulnerabilities section. If it is a genuine concern but you cannot prove exploitability, put it in the Hardening section instead.

4. Separate vulnerabilities from hardening. Vulnerabilities are confirmed exploitable issues rated High, Medium, or Low. Hardening recommendations are defense-in-depth improvements that do not address a specific confirmed vulnerability.

Workflow

1. Determine Scope

Identify what to review:

• If the user specifies files or directories, use those.
• If the user provides a PR number or branch, get the diff:

  git diff main...HEAD --name-only
  

• If no scope is given, ask the user what to review.

Detect the primary language(s) in scope to determine which language-specific checks apply.

Determine what kind of software is being reviewed (CLI tool, library, server, etc.) as this affects how PoCs are written.

2. Enumerate Attack Surface

Before reading code, identify the attack surface by searching for:

net.Listen

http.ListenAndServe

TcpListener::bind

hyper::Server

HandleFunc

Handler

#[get

#[post

axum::Router

actix_web

json.Unmarshal

serde::Deserialize

ParseForm

FromStr

os.Open

os.Create

std::fs::

ioutil.ReadFile

db.Query

db.Exec

sqlx::query

diesel::

exec.Command

std::process::Command

os/exec

crypto/

ring::

sha

hmac

aes

rand

token

jwt

session

auth

permission

rbac

os.Getenv

std::env::var

viper

config

3. Review Code

Read each file in scope. For every file, check against:

1. General security checklist (Section: General Checks)
2. Language-specific checklist (Section: Go Checks or Rust Checks)
3. Context-specific concerns based on the attack surface found in Step 2

For every potential finding, ask yourself:

• Is this reachable from attacker-controlled input? Trace the data flow backwards from the dangerous operation to an input source. Read the actual code at each step. If you cannot trace a path from untrusted input to the dangerous operation, it is not a vulnerability.
• Are there existing controls that mitigate this? Read the surrounding code, middleware, validation layers, and type constraints. If the issue is already mitigated, it is not a vulnerability.
• Can I demonstrate this with a concrete PoC? If not, it belongs in Hardening at most.

4. Investigate Uncertain Findings

For any finding where you are not confident it is exploitable:

Spawn an investigation agent using the Task tool:

Task tool with subagent_type: "general-purpose"

Prompt: "Investigate whether [description of the suspicious pattern] in [file:line]
is actually exploitable. Trace the data flow from any attacker-controlled input to
this operation. Read all relevant validation, middleware, type constraints, and
calling code. Determine:
1. Can an attacker reach this code path with controlled input?
2. Are there existing mitigations that prevent exploitation?
3. If exploitable, what is the concrete attack scenario?
Report back with your conclusion: CONFIRMED, NOT EXPLOITABLE, or NEEDS MORE CONTEXT."

If the agent reports NOT EXPLOITABLE, drop the finding entirely. Do not include it in the report. If CONFIRMED, proceed to write a PoC. If NEEDS MORE CONTEXT, make a judgment call: either investigate further or move it to Hardening with a note about what is uncertain.

5. Write Proof of Concept for Each Vulnerability

Every vulnerability must have a minimal PoC that demonstrates exploitability. The format depends on the type of software:

For a CLI tool:

# PoC: [vulnerability name]
# Expected: [what should happen safely]
# Actual: [what happens due to the vulnerability]
command --flag "$(malicious_payload)"

For a library:

// PoC: [vulnerability name]
package main

import "vulnerable/package"

func main() {
    // Minimal code that triggers the vulnerability
    result := package.VulnerableFunction(maliciousInput)
    // Demonstrate the impact
}

// PoC: [vulnerability name]
use vulnerable_crate::vulnerable_function;

fn main() {
    // Minimal code that triggers the vulnerability
    let result = vulnerable_function(malicious_input);
    // Demonstrate the impact
}

For a server:

# PoC: [vulnerability name]
# Step 1: [setup if needed]
curl -X POST http://localhost:8080/endpoint \
  -H "Content-Type: application/json" \
  -d '{"malicious": "payload"}'
# Expected response: [what safe behavior looks like]
# Actual response: [what the vulnerability produces]

The PoC must be minimal — only the code necessary to trigger the issue and nothing else.

6. Classify Findings

All confirmed vulnerabilities are rated on this scale:

Hardening recommendations are not rated on this scale. They are listed separately without severity ratings.

7. Write Report

Write the report to

SECURITY_REVIEW.md

# Security Review

**Scope:** {files or directories reviewed}
**Date:** {date}
**Languages:** {detected languages}
**Software type:** {CLI tool / library / server / etc.}

## Summary

{1-3 sentences. State the number of confirmed vulnerabilities found and their severity breakdown. If none found, say so clearly.}

---

## Vulnerabilities

{If no vulnerabilities were found, state: "No confirmed vulnerabilities were identified."}

### HIGH-001: {Finding Title}

**File:** `path/to/file.go:42`
**Category:** {e.g., Injection, Authentication, Cryptography}

**Description:**
{What the vulnerability is. Be specific about the data flow: what attacker-controlled input reaches what dangerous operation, and why existing controls do not prevent it.}

**Vulnerable Code:**
{The specific code snippet containing the vulnerability}

**Proof of Concept:**
{Minimal PoC demonstrating the vulnerability — commands, code, or HTTP requests}

**Recommendation:**
{How to fix it, with a corrected code example}

---

### MED-001: {Finding Title}

{same structure}

---

### LOW-001: {Finding Title}

{same structure}

---

## Security Hardening

{Defense-in-depth recommendations that do not address a specific confirmed vulnerability. These are improvements to security posture.}

### {Recommendation Title}

**File:** `path/to/file.go:42` (if applicable)

**Description:**
{What could be improved and why it strengthens security posture, even though it does not address a specific confirmed vulnerability.}

**Suggestion:**
{Concrete code or configuration change}

8. Present to User

After writing the report, summarize the confirmed vulnerabilities inline. Do not summarize hardening items — they are secondary. Ask if the user wants to discuss any specific finding in detail.

General Checks

Apply these checks regardless of language. Remember: only flag items where you can trace attacker-controlled input to a dangerous operation without existing mitigation.

Input Validation

• Untrusted input used directly in SQL queries, commands, file paths, or URLs
• Missing or insufficient length/size limits on inputs
• Improper handling of Unicode, null bytes, or encoding edge cases
• Path traversal via

  ../

  or absolute paths in user-supplied filenames
• SSRF via user-controlled URLs used in outbound requests

Authentication and Authorization

• Missing authentication on sensitive endpoints
• Authorization checks that can be bypassed (IDOR, missing ownership checks)
• Hard-coded credentials, API keys, or tokens
• Timing-safe comparison not used for secrets (

  ==

  instead of constant-time compare)
• Session tokens with insufficient entropy or predictable generation

Cryptography

• Use of broken algorithms (MD5, SHA1 for security purposes, DES, RC4)
• Hard-coded keys, IVs, or nonces
• Nonce reuse in AEAD ciphers
• Use of ECB mode
• Custom cryptographic implementations instead of vetted libraries
• Insufficient key lengths (RSA < 2048, symmetric < 128-bit)
• Missing integrity protection (encryption without authentication)

Error Handling

• Stack traces, internal paths, or debug info exposed to users
• Errors that reveal whether a user/resource exists (enumeration)
• Catch-all error handlers that swallow security-relevant failures
• Panics or crashes reachable from untrusted input

Secrets Management

• Secrets logged to stdout/stderr or log files
• Secrets in source code, config files committed to VCS, or environment variable defaults
• Secrets not zeroed from memory after use

• .env

  files, private keys, or credential files in the repository

Concurrency

• Race conditions in authentication or authorization checks (TOCTOU)
• Shared mutable state without synchronization
• Double-spend or duplicate-action vulnerabilities from concurrent requests

Dependency and Supply Chain

• Dependencies with known CVEs (check go.sum, Cargo.lock)
• Pinned to mutable refs (branches,

  latest

  ) instead of exact versions or hashes
• Vendored code that has been modified from upstream

Go Checks

Apply these when reviewing Go code.

Injection

• fmt.Sprintf

  used to build SQL queries instead of parameterized queries

• os/exec.Command

  with user-controlled arguments not validated against an allowlist

• text/template

  used where

  html/template

  is needed (XSS)
• String concatenation in

  database/sql

  query construction

Error Handling

• Errors discarded with

  _ =

  on security-critical operations (file permissions, crypto, auth)

• defer

  closing a resource but ignoring the close error (can mask write failures)

• recover()

  catching panics broadly, masking security failures
• Missing error checks on

  io.Copy

  ,

  http.Response.Body.Close

HTTP and Networking

• Missing TLS configuration or insecure

  TLSClientConfig

  (

  InsecureSkipVerify: true

  )

• http.DefaultClient

  used without timeouts (enables slowloris DoS)
• Missing

  http.MaxBytesReader

  on request bodies (memory exhaustion)
• CORS headers set to

  *

  or reflecting origin without validation
• Missing CSRF protection on state-changing endpoints

• http.Redirect

  with user-controlled URL (open redirect)

Concurrency

• Race conditions on maps (concurrent read/write without

  sync.Map

  or mutex)
• Goroutine leaks from missing context cancellation or unbounded channel sends

• sync.WaitGroup

  misuse (Add/Done mismatch)
• Shared slice/map modified across goroutines without synchronization

Unsafe Operations

• Use of

  unsafe

  package for pointer arithmetic or type punning

• reflect

  used to bypass unexported field protections

• cgo

  calls with unchecked buffer sizes or missing null termination

• //go:linkname

  used to access internal runtime functions

Cryptographic Pitfalls

• math/rand

  used where

  crypto/rand

  is required (token generation, nonces)

• crypto/subtle.ConstantTimeCompare

  not used for secret comparison
• Custom

  hash.Hash

  usage without proper reset between uses

• crypto/tls

  config missing

  MinVersion: tls.VersionTLS12

Integer and Memory

• Integer overflow in

  make([]byte, userControlledSize)

  causing allocation issues
• Slice bounds not checked before indexing with external values

• strconv.Atoi

  result used without range validation

File and OS

• os.MkdirAll

  with overly permissive mode (0777)

• os.OpenFile

  without validating symlink targets (symlink following)
• Temporary files created in shared directories without

  os.CreateTemp

• Missing

  filepath.Clean

  on user-supplied paths

Rust Checks

Apply these when reviewing Rust code.

Unsafe Code

• Every

  unsafe

  block: verify the safety invariants documented and upheld
• Raw pointer dereferencing without null/alignment checks

• std::mem::transmute

  used for type punning (verify layout compatibility)

• unsafe impl Send/Sync

  on types containing raw pointers or non-thread-safe internals

• from_raw_parts

  /

  from_raw_parts_mut

  with unchecked length or alignment

• ManuallyDrop

  or

  std::mem::forget

  causing resource leaks (file handles, locks)
• FFI boundaries: missing null checks, buffer size validation, lifetime guarantees

Error Handling

• .unwrap()

  or

  .expect()

  on

  Result

  /

  Option

  reachable from untrusted input

• panic!

  in library code reachable from external callers

• ?

  propagation that discards context needed for security decisions

• catch_unwind

  used as a substitute for proper error handling

Memory Safety (even in safe Rust)

• Logic errors in index arithmetic leading to out-of-bounds via

  .get_unchecked()

• Vec::set_len()

  on uninitialized memory
• Stack overflow via unbounded recursion on attacker-controlled input
• Large allocations from user-controlled sizes (

  Vec::with_capacity(user_input)

  )

Serialization and Parsing

• serde

  deserialization of untrusted input without size limits or depth limits

• #[serde(deny_unknown_fields)]

  missing on security-critical config structs
• Custom

  Deserialize

  implementations with unchecked invariants
• Deserializing into

  enum

  variants that trigger different privilege levels

Web and Network (axum, actix-web, hyper, tokio)

• Missing request body size limits (

  tower_http::limit::RequestBodyLimitLayer

  )
• Timeout not configured on server or client connections
• Missing TLS certificate validation on HTTP clients (

  danger_accept_invalid_certs

  )
• Shared mutable state in handlers without

  Arc<Mutex<_>>

  or

  Arc<RwLock<_>>

• Missing authentication middleware on protected routes

• tower

  layers ordered incorrectly (auth after handler, rate limit after auth)

Concurrency

• Mutex

  poisoning not handled (

  lock().unwrap()

  on potentially poisoned mutex)

• Arc

  reference cycles causing memory leaks
• Deadlock potential from inconsistent lock ordering

• tokio::spawn

  without

  JoinHandle

  tracking (silent task failures)
• Blocking operations in async context (

  std::fs

  in tokio runtime,

  std::thread::sleep

  )

Cryptographic Pitfalls

• rand::thread_rng()

  used where

  rand::rngs::OsRng

  is needed for cryptographic randomness

• ring

  or

  rustcrypto

  primitives used without authenticated encryption
• Custom serialization of cryptographic keys without constant-time comparison
• Timing side-channels in comparison operations (

  ==

  on secrets)

Supply Chain and Build

• build.rs

  executing arbitrary code at compile time

• proc_macro

  dependencies with network access or file system writes

• [patch]

  or

  [replace]

  in

  Cargo.toml

  overriding dependencies
• Feature flags that silently disable security features (

  #[cfg(not(feature = "tls"))]

  )

Integer Safety

• Arithmetic operations that can overflow/underflow (use

  checked_*

  ,

  saturating_*

  , or

  wrapping_*

  )

• as

  casts that truncate or sign-extend (

  u64 as u32

  ,

  i32 as u32

  )

• usize

  used for serialized data sizes across architectures (32 vs 64-bit mismatch)