首頁 / Skills / 開發提效 / 安全與加固 Security and Hardening
開發提效

安全與加固 Security and Hardening

Addy OsmaniMITSkill 語言: 英文
授權已核對不含腳本低風險
演示真實執行紀錄 · claude-sonnet-5-5 · 2026年10月
Please review this Express handler for security problems and show me the fixed version. It is a login plus avatar-upload endpoint in a small web app (sample code, not a real system). ```js const express = require('express'); const cors = require('cors'); const app = express(); app.use(cors({ origin: '*', credentials: true })); app.use(express.json()); app.post('/login', async (req, res) => { const { email, password } = req.body; const user = await db.query(`SELECT * FROM users WHERE email = '${email}'`); if (user.rows[0] && user.rows[0].password === password) { const token = jwt.sign({ id: user.rows[0].id }, 'secret123'); res.json({ token, user: user.rows[0] }); } else { res.status(401).json({ error: 'bad login', detail: `no user ${email}` }); } }); app.post('/avatar', upload.single('file'), (req, res) => { fs.writeFileSync('/var/www/uploads/' + req.file.originalname, req.file.buffer); res.send('ok'); }); app.get('/profile/:id', async (req, res) => { res.json((await db.query('SELECT * FROM users WHERE id = ' + req.params.id)).rows[0]); }); // the front end stores the token in localStorage ```
Skill: security-and-hardening

Review: security problems found

# Severity Problem
1 Critical SQL injection in /login and /profile/:id, from string-concatenated queries.
2 Critical Path traversal and arbitrary file overwrite in /avatar. originalname is attacker-controlled, so ../../x escapes the folder.
3 Critical Uploads are unauthenticated and unrestricted. There is no size, type, or content check. The files go into /var/www, so an uploaded .html or .svg gives stored XSS, and a script file may get executed.
4 High Plaintext password comparison. Passwords are stored unhashed and compared with ===, which is also not timing-safe.
5 High Hardcoded JWT secret ('secret123'). There is no expiry and the algorithm isn't pinned, so anyone can forge tokens.
6 High Full DB row returned (SELECT *) from /login and /profile. This leaks the password and any other sensitive columns.
7 High /profile/:id has no authentication or authorization (IDOR). Anyone can read any user.
8 High Token in localStorage. Any XSS can steal it. Use an httpOnly cookie instead.
9 Medium cors({ origin: '*', credentials: true }) is a misconfiguration. Use an explicit allowlist.
10 Medium The 401 reply echoes the email (detail). That is reflected input and it enables user enumeration.
11 Medium No rate limiting on login, so brute force and credential stuffing are open.
12 Medium No input validation. email could be an object or array, not just a string.
13 Low No security headers (helmet).
14 Low Async errors are unhandled in Express 4, which gives hangs or stack traces. writeFileSync also blocks the event loop.

Fixed version

const express = require('express');
const cors = require('cors');
const helmet = require('helmet');
const cookieParser = require('cookie-parser');
const rateLimit = require('express-rate-limit');
const bcrypt = require('bcrypt');
const jwt = require('jsonwebtoken');
const multer = require('multer');
const crypto = require('crypto');
const path = require('path');
const fs = require('fs/promises');
const { z } = require('zod');

const JWT_SECRET = process.env.JWT_SECRET;
if (!JWT_SECRET || JWT_SECRET.length < 32) throw new Error('JWT_SECRET missing or too short');

const ALLOWED_ORIGINS = (process.env.CORS_ORIGINS || '').split(',').filter(Boolean);
const UPLOAD_DIR = path.resolve(process.env.UPLOAD_DIR || '/var/app-data/avatars'); // outside the web root
const DUMMY_HASH = bcrypt.hashSync('dummy-password', 12); // equalizes timing for unknown emails

const app = express();
app.use(helmet());
app.use(cors({ origin: ALLOWED_ORIGINS, credentials: true }));
app.use(express.json({ limit: '10kb' }));
app.use(cookieParser());

const wrap = fn => (req, res, next) => fn(req, res, next).catch(next);

// ---- auth middleware ----
function requireAuth(req, res, next) {
  try {
    const payload = jwt.verify(req.cookies.session, JWT_SECRET, { algorithms: ['HS256'] });
    req.userId = payload.id;
    next();
  } catch {
    res.status(401).json({ error: 'unauthorized' });
  }
}

// ---- login ----
const loginLimiter = rateLimit({ windowMs: 15 * 60 * 1000, max: 10 }); // use a shared store (e.g. Redis) if you run >1 instance
const loginSchema = z.object({ email: z.string().email().max(254), password: z.string().min(1).max(200) });

app.post('/login', loginLimiter, wrap(async (req, res) => {
  const parsed = loginSchema.safeParse(req.body);
  if (!parsed.success) return res.status(422).json({ error: 'invalid input' });
  const { email, password } = parsed.data;

  const { rows } = await db.query(
    'SELECT id, email, display_name, password_hash FROM users WHERE email = $1', [email]);
  const user = rows[0];
  const ok = await bcrypt.compare(password, user ? user.password_hash : DUMMY_HASH);
  if (!user || !ok) return res.status(401).json({ error: 'invalid email or password' });

  const token = jwt.sign({ id: user.id }, JWT_SECRET, { algorithm: 'HS256', expiresIn: '1h' });
  res.cookie('session', token, {
    httpOnly: true, secure: true, sameSite: 'strict', maxAge: 60 * 60 * 1000,
  });
  res.json({ user: { id: user.id, email: user.email, displayName: user.display_name } });
}));

// ---- avatar upload ----
const upload = multer({
  storage: multer.memoryStorage(),
  limits: { fileSize: 2 * 1024 * 1024, files: 1 },
});

// verify real content via magic bytes; the extension and client MIME type prove nothing
function sniffImage(buf) {
  if (buf.length > 8 && buf.subarray(0, 8).equals(Buffer.from([0x89,0x50,0x4e,0x47,0x0d,0x0a,0x1a,0x0a]))) return 'png';
  if (buf.length > 3 && buf[0] === 0xff && buf[1] === 0xd8 && buf[2] === 0xff) return 'jpg';
  return null; // SVG and HTML deliberately not allowed
}

app.post('/avatar', requireAuth, upload.single('file'), wrap(async (req, res) => {
  if (!req.file) return res.status(422).json({ error: 'file required' });
  const ext = sniffImage(req.file.buffer);
  if (!ext) return res.status(415).json({ error: 'only PNG or JPEG allowed' });

  // server-chosen name: the client filename is never used
  const name = `${req.userId}-${crypto.randomUUID()}.${ext}`;
  const dest = path.join(UPLOAD_DIR, name);
  if (path.dirname(dest) !== UPLOAD_DIR) throw new Error('bad path'); // defense in depth
  await fs.writeFile(dest, req.file.buffer, { flag: 'wx', mode: 0o640 });

  await db.query('UPDATE users SET avatar_file = $1 WHERE id = $2', [name, req.userId]);
  res.json({ ok: true });
}));
// Serve avatars via a static handler or CDN with `X-Content-Type-Options: nosniff`
// and a fixed image Content-Type.

// ---- profile ----
app.get('/profile/:id', requireAuth, wrap(async (req, res) => {
  const id = Number.parseInt(req.params.id, 10);
  if (!Number.isSafeInteger(id)) return res.status(422).json({ error: 'invalid id' });
  if (id !== req.userId) return res.status(403).json({ error: 'forbidden' }); // add an admin check if needed

  const { rows } = await db.query(
    'SELECT id, email, display_name, avatar_file FROM users WHERE id = $1', [id]);
  if (!rows[0]) return res.status(404).json({ error: 'not found' });
  res.json(rows[0]);
}));

// ---- error handler: details go to logs, not to the client ----
app.use((err, req, res, next) => {
  console.error(err);
  res.status(500).json({ error: 'internal error' });
});

Notes on the changes

  • Passwords: your table needs a password_hash column. Migrate existing users by hashing their passwords with bcrypt (cost 12 or higher) or forcing a reset. Never keep the plaintext column.
  • Token storage: moving from localStorage to an httpOnly cookie means the front end must call fetch(..., { credentials: 'include' }) and stop touching the token. sameSite: 'strict' and the CORS allowlist cover CSRF for this setup. If the front end runs on a different site (not just a different subdomain), add CSRF tokens.
  • Secrets: JWT_SECRET comes from the environment. Treat secret123 as compromised and rotate it, since any tokens signed with it can be forged.
  • Rate limiting: the in-memory limiter works for a single process. With multiple instances, back it with Redis or similar, or the effective limit is 10 × instances.
  • Not covered: db, upload, jwt and fs were undefined in your sample, so I assumed pg-style $1 placeholders. I also didn't add account lockout, password policy, or logging of failed logins. Those are worth adding for a real system.

Test run: a deliberately vulnerable sample Express handler (login, upload, profile). The model listed 14 problems by severity and gave a fixed version. The fixed code was not run, and the model noted it assumed a pg-style database client. It did not open the bundled reference files in this run.

它做什麼

把每個外部輸入都當作有敵意的,把每個金鑰都當作機密。它先做一次簡短的威脅建模(畫出信任邊界、列出資產、過一遍 STRIDE、在使用案例旁寫濫用案例),然後套用三層規則:一律做(驗證輸入、參數化查詢、輸出編碼、雜湊密碼、安全的 Cookie、執行套件管理員內建的稽核),先問再做(新增認證流程、新增敏感資料、改 CORS 或上傳),絕不做(提交金鑰、記錄敏感資料、把用戶端驗證當安全邊界)。涵蓋注入、XSS、存取控制(IDOR)、工作階段、安全回應標頭、CORS、檔案上傳、SSRF、由推導路徑觸發的破壞性操作、限流、金鑰、相依套件與供應鏈檢查、個人資料與隱私,以及 AI 功能(模型輸出同樣視為不可信輸入)。

輸出

附嚴重程度的問題清單、修正後的程式碼,以及需要遷移或繼續檢查的事項。

適合什麼場景

撰寫或審查會接收輸入、處理帳號、儲存資料或呼叫第三方服務的程式碼的人。套件內附有 Skill 引用的模式參考與共用的安全檢查清單。

說明與風險

低風險:純指令檔,沒有腳本。它會讓智能體修改你的程式碼,並執行你的套件管理員內建的稽核,但不會自動執行強制修復。已用一個故意寫得有漏洞的範例介面試用一次:找出 14 個問題並給出修正版本,修正後的程式碼沒有執行;那次模型沒有開啟套件內的參考檔。它是指引,不是安全稽核,也不是保證。共用的檢查清單已一起打包並修正路徑,確保引用找得到。作者 Addy Osmani,MIT 授權。

SKILL.md 原文套件內共 4 個檔案