白盒代码安全审计 — Cloudflare security-audit-skill 六阶段产出

Phase 1 · architecture.md

Recon output. Application shape, trust boundaries, complete HTTP input surface, and the risk-ranked hotspots that seeded the Phase 2 hunters.

What this is

OWASP NodeGoat — an intentionally-vulnerable retirement-plan web app maintained as a teaching artifact for the OWASP Top 10. Node 12 + Express 4, MongoDB persistence, swig templates via consolidate, express-session cookies, needle for outbound HTTP.

• Entry: server.js — boots Mongo, wires middleware, mounts routes.
• Routes: app/routes/*.js (session, profile, contributions, allocations, benefits, memos, research, index, tutorial, error).
• Data access: app/data/*-dao.js.
• Views: app/views/*.html (swig templates).
• Config: config/env/*.json merged by config/config.js.

Comparable class: prior-generation intentionally-vulnerable web apps (DVWA / WebGoat / Juice Shop). Findings are calibrated as real vulnerabilities against the running behavior, not as a checklist of deviations.

Trust boundaries

Input surfaces (HTTP)

Auth model

• Password storage: plaintext. app/data/user-dao.js:25 writes the raw password field on the user document; the bcrypt.hashSync(...) fix is present commented at lines 27-30.
• Password comparison: non-constant-time ===. user-dao.js:61. The bcrypt.compareSync(...) fix is commented at line 65.
• Session id: assigned on successful login by session.js:116 without calling req.session.regenerate(...). The correct pattern is used at signup (session.js:234) — login is a genuine gap.
• Admin gate: isAdminUserMiddleware exists at session.js:25-34 but is never mounted. /benefits uses only isLoggedIn (index.js:55-56).

Template engine + escaping

• swig via consolidate (server.js:116-118).
• autoescape disabled globally (server.js:135-137). Any {{ variable }} renders unescaped unless the template pipes through an explicit filter.
• marked.setOptions({ sanitize: true }) is set at server.js:126-128, but sanitize was deprecated / removed in later marked versions.

Deployment surface

• http.createServer(app).listen(port) at server.js:145 — HTTP only; HTTPS block commented at lines 20-27 / 149-155.
• helmet and dont-sniff-mimetype imported-then-commented at server.js:10-14 / 40-64; no CSP, HSTS, X-Frame-Options, MIME-sniff protection.
• csurf likewise imported-then-commented at server.js:7 / 104-113; every state-changing POST accepts requests with no CSRF token.

Risk hotspots (highest hunting value)

1. app/routes/contributions.js:32-34 — three literal eval(req.body.*) calls. SSJS injection with no role restriction.
2. app/data/allocations-dao.js:77-79 — MongoDB $where clause built with template-string interpolation of req.query.threshold.
3. app/routes/allocations.js:16-18 — userId from req.params instead of req.session.
4. app/routes/index.js:55-56 — /benefits mount without the isAdmin middleware.
5. app/routes/research.js:14-16 — needle.get(req.query.url + req.query.symbol).
6. app/routes/index.js:70-73 — res.redirect(req.query.url).
7. app/routes/profile.js:59 — regex /([0-9]+)+\#/ on user input; textbook ReDoS.
8. app/data/user-dao.js:25 + :61 — plaintext password storage and non-constant-time compare at the same site.
9. app/routes/session.js:116 — session id not regenerated at login.

Baseline / calibration

This code is intentionally vulnerable. The auditor's job here is NOT to enumerate every intentional gap (that reproduces the OWASP tutorial and produces a useless report), but to (a) confirm each intentional gap is actually exploitable in the current source and (b) rank findings by concrete attacker impact so the report reads like a real audit and not a study guide. Findings without a demonstrable payload against the current source were dropped from the report.

Phase 4 · REPORT.md

Human-readable audit report. Severity distribution, ranked findings with two-line summaries, plus one call-out for what the codebase does well.

Severity summary

Findings

What NodeGoat does well (in this run)

• app/data/user-dao.js:91-93 uses a typed field lookup (findOne({userName: userName})) rather than string-concatenated queries, so login is not NoSQL-injectable even though the rest of the codebase is.
• Signup (session.js:234) correctly wraps req.session.userId = user._id in req.session.regenerate(...). It is only login that omits regeneration.

Coverage note

This is run-1 against this target. Cloudflare's harness data suggests a single run of this skill finds roughly half of what is present across multiple runs. Recommend a run-2 focused on: prototype pollution in body-parsed objects, tutorial-router path traversal (app/routes/tutorial.js), the memos template rendering path (stored XSS via swig autoescape=false), and business-logic edge cases in contributions.js:47-57 after the eval issue is fixed.

Phase 4 · FINDINGS-DETAIL.md

MEDIUM+ findings with full data-flow trace (entrypoint → propagation → sink), reproducer payload, expected result, and remediation. Excerpt of the top three findings shown; the file on disk contains all nine.

POST /contributions HTTP/1.1
Cookie: connect.sid=<any authenticated session>
Content-Type: application/x-www-form-urlencoded

preTax=require('child_process').execSync('id').toString()&afterTax=0&roth=0

GET /allocations/1?threshold=0'%20%7C%7C%20'1'%3D%3D'1
    -> decoded: 0' || '1'=='1
    -> effective $where: this.userId == 1 && this.stocks > '0' || '1'=='1'
    -> matches every document; exfiltrates all allocations.

GET /allocations/1?threshold=0'%3B%20while(true)%7B%7D%2F%2F
    -> decoded: 0'; while(true){}//
    -> hangs the MongoDB collection scan indefinitely (DoS).

GET /allocations/2   ← as logged-in user 1
Cookie: connect.sid=<any authenticated session>

Findings §4-9 (missing admin authorization, plaintext passwords, SSRF, open redirect, ReDoS, session fixation) follow the same shape — each has an entrypoint → propagation → sink trace, at least one concrete payload, expected result, and a code-change remediation. See the full FINDINGS-DETAIL.md on disk.

Phase 5 · findings.json (schema-validated)

Machine-readable output conforming to the upstream report-schema.json. Every finding has a trace array whose first step is kind: entrypoint, last is kind: sink, intermediate steps are kind: propagation — enforced semantically by validate-findings.cjs.

Validator

node validate-findings.cjs findings.json — zero dependencies

Result

PASS   9/9 findings valid; exit code 0

Severity mix

2 critical · 5 high · 2 medium

Schema

oneOf: [confirmed, rejected]; every confirmed finding requires verdict, title, description, root_cause, intended_behavior, trace, conditions, execution, remediation, severity, confidence.

Excerpt — finding [0] (Critical: SSJS Injection)

{
  "verdict": "confirmed",
  "title": "Server-Side JavaScript Injection via eval() in POST /contributions",
  "description": "The POST /contributions handler runs eval() on three raw request-body fields (preTax, afterTax, roth) before any sanitization. ...",
  "root_cause": "handleContributionsUpdate in app/routes/contributions.js does not parse the request body as numeric input, allowing eval to execute arbitrary JavaScript with the Node process's privileges.",
  "intended_behavior": "The handler is meant to accept three percentage integers ... The commented block at lines 36-41 shows the maintainers know the safe form is parseInt(req.body.<field>, 10).",
  "trace": [
    { "kind": "entrypoint",  "file": "app/routes/contributions.js", "line": 28, "scope": "handleContributionsUpdate", "description": "Express route registered at index.js:52 invokes this handler; req.body is populated by bodyParser." },
    { "kind": "propagation", "file": "app/routes/contributions.js", "line": 32, "scope": "handleContributionsUpdate", "description": "const preTax = eval(req.body.preTax); the raw string from the body is passed to eval." },
    { "kind": "propagation", "file": "app/routes/contributions.js", "line": 33, "scope": "handleContributionsUpdate", "description": "const afterTax = eval(req.body.afterTax); same pattern on the second field." },
    { "kind": "sink",        "file": "app/routes/contributions.js", "line": 34, "scope": "handleContributionsUpdate", "description": "const roth = eval(req.body.roth); third eval; any of the three eval calls executes attacker-supplied JavaScript." }
  ],
  "conditions": [
    { "kind": "authentication_level", "description": "Requires any authenticated session (isLoggedIn middleware only); no admin role or CSRF token required." }
  ],
  "execution": {
    "attacker_perspective": "Any signed-in user of the NodeGoat instance.",
    "payloads": [
      "preTax=require('child_process').execSync('id').toString()&afterTax=0&roth=0",
      "preTax=process.exit(1)&afterTax=0&roth=0"
    ],
    "instructions": [
      "Register or log in and capture the connect.sid cookie.",
      "Submit POST /contributions with body preTax=<payload>&afterTax=0&roth=0.",
      "Observe the eval side effect on the server."
    ],
    "expected_result": "The eval'd JavaScript expression executes with the privileges of the Node.js process; the HTTP response ends in the standard 'Invalid contribution percentages' render because isNaN of the return value is true, but the side effect has already occurred."
  },
  "remediation": {
    "strategy": "Replace all three eval(req.body.<field>) calls with parseInt(req.body.<field>, 10). The commented block at lines 36-41 already contains the fix verbatim.",
    "code_changes": [
      { "file_name": "app/routes/contributions.js",
        "fixed_code": "const preTax = parseInt(req.body.preTax, 10);\nconst afterTax = parseInt(req.body.afterTax, 10);\nconst roth = parseInt(req.body.roth, 10);" }
    ]
  },
  "severity": {
    "likelihood": { "score": "high", "reason": "Requires only a valid session; the attack is a single POST with a trivial payload." },
    "impact":     { "score": "critical", "reason": "Server-side arbitrary code execution as the Node process." },
    "overall_severity": "critical"
  },
  "confidence": {
    "score": "high",
    "reason": "eval on req.body is present in three literal calls at lines 32-34; the route mount at app/routes/index.js:52 is direct; verified by re-reading both files."
  }
}

Validator run

$ node validate-findings.cjs findings.json
Checking [0] Server-Side JavaScript Injection via eval() in POST /contributions
Checking [1] NoSQL injection via $where operator in GET /allocations/:userId
Checking [2] Insecure Direct Object Reference on GET /allocations/:userId
Checking [3] Missing admin authorization on /benefits allows non-admin benefit mutation
Checking [4] Plaintext password storage with non-constant-time compare
Checking [5] Server-Side Request Forgery in GET /research
Checking [6] Open redirect on GET /learn via res.redirect(req.query.url)
Checking [7] Regex Denial of Service on POST /profile bankRouting field
Checking [8] Session fixation on POST /login (no req.session.regenerate)

PASS: 9 findings valid

Findings [1] through [8] follow the same shape — each carries a full trace, execution, remediation, severity, and confidence block. The complete findings.json on disk is the machine-readable source of truth downstream tools consume.