홈 / 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 점검, 사용 사례 옆에 악용 사례 쓰기)을 하고, 3단계 규칙을 적용합니다. 항상 한다(입력 검증, 쿼리 매개변수화, 출력 인코딩, 비밀번호 해시, 안전한 쿠키, 패키지 관리자의 감사 실행), 먼저 묻는다(새 인증 흐름, 새 민감 데이터, CORS나 업로드 변경), 절대 하지 않는다(비밀 정보 커밋, 민감 데이터 로깅, 클라이언트 검증을 보안 경계로 믿기)입니다. 인젝션, XSS, 접근 제어(IDOR), 세션, 보안 헤더, CORS, 파일 업로드, SSRF, 유도된 경로에 대한 파괴적 작업, 요청 제한, 비밀 정보, 의존성과 공급망 점검, 개인 데이터와 프라이버시, AI 기능(모델 출력도 신뢰할 수 없는 입력)을 다룹니다.

결과물

심각도가 붙은 문제 목록, 고친 코드, 이전하거나 더 확인할 사항.

이런 때 좋습니다

입력을 받거나, 계정을 다루거나, 데이터를 저장하거나, 외부 서비스를 호출하는 코드를 쓰거나 검토하는 사람. 이 Skill이 참조하는 패턴 모음과 공통 보안 체크리스트도 함께 들어 있습니다.

참고 사항 및 위험

낮은 위험:지침만 있는 파일로 스크립트가 없습니다. 에이전트가 코드를 고치고 패키지 관리자의 감사를 실행하게 하지만 강제 수정은 자동으로 적용하지 않습니다. 일부러 취약하게 만든 예제 핸들러로 한 번 시험했고 14개 문제를 찾아 수정본을 줬으며(수정된 코드는 실행하지 않음), 그때 모델은 포함된 참고 파일을 열지 않았습니다. 이는 지침일 뿐 보안 감사나 보증이 아닙니다. 공통 체크리스트도 함께 넣고 경로를 고쳐 참조가 연결되도록 했습니다. 작성자 Addy Osmani, MIT.

SKILL.md 원본패키지에 파일 4개