首页 / 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 个文件