ホーム / 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 ファイル