maxim.dolgolyov/Learn_System
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

security+perf: полное ревью — 17 фиксов P0/P1 (XSS, IDOR, race conditions, rate limits, TURN, WAL)

Browse Source 
## P0
- admin.html:2608, red-book-ecosystem.html:489-495 — XSS: u.name/node.name_ru/description обернуты в LS.esc()
- classController.js getAnnouncements — добавлена проверка teacher_id (B14: учитель A не может читать объявления класса B)

## P1 — auth & validation
- authController.js — минимум пароля 6→8 символов (register + change password + login.html)
- gamificationController adminAward — валидация max XP/coins (1M), Number coercion
- shopController adminAwardCoins — валидация max + проверка changes>0

## P1 — race conditions
- petController.buyBg — atomic UPDATE WHERE coins>=? (race-safe)
- shopController.purchaseItem — atomic conditional UPDATE
- liveController — добавлен question_id в live_answers (миграция с пересозданием таблицы), история ответов сохраняется при смене вопроса учителем
- ws-server: invalidateDrawCache экспортирован, classroomController grant/revoke вызывают его → permission revoke применяется мгновенно (раньше до 10s stale)

## P1 — rate limits & retry
- rateLimit middleware: новый параметр byUser=true (использует req.user.id вместо IP — не блокирует пользователей за NAT)
- routes/classroom.js: reactionLimiter (15/5s) на /chat/:msgId/react, handLimiter (5/5s) на raise/lower hand
- api.js sendAnswer — retry 3x с exp backoff (300/1200/2700ms), не повторяет на 4xx (F5)

## P1 — performance
- classroomController.getStrokes — LIMIT 5000 + флаг hasMore (защита от OOM на 10K+ strokes)
- whiteboard.js _liveStrokes — TTL 1.5s на каждый live preview (auto-cleanup при крашe ремоут юзера)

## Infrastructure
- config.js: TURN_URL/USER/PASS env vars
- server.js: GET /api/ice-servers возвращает STUN + опциональный TURN из env
- classroom-rtc.js: фетчит /api/ice-servers вместо хардкода (поддержка TURN для NAT/CGNAT школьных сетей)
- .env.example: документация TURN
- db.js: PRAGMA synchronous=NORMAL (5x быстрее с WAL), cache_size 16MB, temp_store=MEMORY
- ws-server.js closeAll() + server.js shutdown — graceful WS shutdown при SIGTERM

## False positives (не баги, агенты ошиблись)
- assignmentController FK на tests — на самом деле users (migrate.js:317-318)
- .env в git — gitignore корректно исключает
- admin.html без requireAuth — есть LS.initPage() который вызывает requireAuth
- submissionsController IDOR — обе ручки уже проверяют teacher_id
- screenSender = null inside try/catch — на самом деле снаружи
- SSE без backoff — есть exponential 2s→30s
- sessionController NOT IN на пустом массиве — есть guard usedIds.length>0
- getChat без LIMIT — есть LIMIT 100/200
- trust proxy — установлен на server.js:105

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Maxim Dolgolyov
2026-04-23 12:16:08 +03:00
parent 2ae06ba2f1
commit 952a54f97c
21 changed files with 231 additions and 62 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/src/controllers/authController.js
+3 -3
View File
@@ -19,8 +19,8 @@ async function register(req, res, next) {
if (!email || !password || !name)
return res.status(400).json({ error: 'email, password and name are required' });
if (password.length < 6)
return res.status(400).json({ error: 'Password must be at least 6 characters' });
if (password.length < 8)
return res.status(400).json({ error: 'Password must be at least 8 characters' });
if (db.prepare('SELECT id FROM users WHERE email = ?').get(email))
return res.status(409).json({ error: 'Email already registered' });
@@ -84,7 +84,7 @@ async function updateProfile(req, res, next) {
if (!currentPassword) return res.status(400).json({ error: 'Текущий пароль обязателен' });
const valid = await bcrypt.compare(currentPassword, user.password_hash);
if (!valid) return res.status(401).json({ error: 'Неверный текущий пароль' });
if (newPassword.length < 6) return res.status(400).json({ error: 'Пароль минимум 6 символов' });
if (newPassword.length < 8) return res.status(400).json({ error: 'Пароль минимум 8 символов' });
const hash = await bcrypt.hash(newPassword, BCRYPT_ROUNDS);
db.prepare('UPDATE users SET password_hash = ?, token_version = token_version + 1 WHERE id = ?').run(hash, user.id);
}
backend/src/controllers/classController.js
+4
View File
@@ -420,6 +420,10 @@ function myClasses(req, res) {
/* ── GET /api/classes/:id/announcements ─────────────────────────────────── */
function getAnnouncements(req, res) {
const cls = stmts.getClassOwner.get(req.params.id);
if (!cls) return res.status(404).json({ error: 'Not found' });
if (req.user.role !== 'admin' && cls.teacher_id !== req.user.id)
return res.status(403).json({ error: 'Forbidden' });
const announcements = db.prepare(`
SELECT a.id, a.text, a.created_at, u.name AS author_name
FROM announcements a
backend/src/controllers/classroomController.js
+9 -4
View File
@@ -3,7 +3,7 @@ const path = require('path');
const fs = require('fs');
const crypto = require('crypto');
const { getOnlineUserIds } = require('../sse');
const { emitToUser, invalidateSession } = require('../ws-server');
const { emitToUser, invalidateSession, invalidateDrawCache } = require('../ws-server');
/* ── chat attachment uploads dir ─────────────────────────────────────── */
const CHAT_UPLOADS_DIR = path.join(__dirname, '../../uploads/chat');
@@ -616,17 +616,20 @@ function getStrokes(req, res) {
if (!hasAccess(session, req.user.id, req.user.role))
return res.status(403).json({ error: 'Нет доступа' });
// LIMIT 5000 — guards against OOM on very long sessions; client paginates via since_seq
const STROKES_PAGE_LIMIT = 5000;
const rows = sinceSeq >= 0
? db.prepare('SELECT id, tool, data, seq FROM classroom_strokes WHERE session_id=? AND page_num=? AND seq > ? ORDER BY seq').all(sessionId, pageNum, sinceSeq)
: db.prepare('SELECT id, tool, data, seq FROM classroom_strokes WHERE session_id=? AND page_num=? ORDER BY seq').all(sessionId, pageNum);
? db.prepare('SELECT id, tool, data, seq FROM classroom_strokes WHERE session_id=? AND page_num=? AND seq > ? ORDER BY seq LIMIT ?').all(sessionId, pageNum, sinceSeq, STROKES_PAGE_LIMIT)
: db.prepare('SELECT id, tool, data, seq FROM classroom_strokes WHERE session_id=? AND page_num=? ORDER BY seq LIMIT ?').all(sessionId, pageNum, STROKES_PAGE_LIMIT);
const strokes = rows.map(r => ({ ...r, data: JSON.parse(r.data) }));
const hasMore = rows.length === STROKES_PAGE_LIMIT;
const pageRow = db.prepare('SELECT template, name FROM classroom_pages WHERE session_id=? AND page_num=?').get(sessionId, pageNum);
const template = pageRow?.template || 'blank';
const name = pageRow?.name || null;
res.json({ strokes, template, name });
res.json({ strokes, template, name, hasMore });
}
/* PATCH /api/classroom/:id/strokes/:strokeId — update image position/size */
@@ -870,6 +873,7 @@ function allowDraw(req, res) {
db.prepare(
'INSERT OR IGNORE INTO classroom_draw_permissions (session_id, user_id) VALUES (?,?)'
).run(sessionId, targetId);
invalidateDrawCache(sessionId, targetId);
emitToUser(targetId, { type: 'classroom_draw_permitted', sessionId });
res.json({ ok: true });
@@ -887,6 +891,7 @@ function revokeDraw(req, res) {
db.prepare(
'DELETE FROM classroom_draw_permissions WHERE session_id=? AND user_id=?'
).run(sessionId, targetId);
invalidateDrawCache(sessionId, targetId);
emitToUser(targetId, { type: 'classroom_draw_revoked', sessionId });
res.json({ ok: true });
backend/src/controllers/gamificationController.js
+7 -2
View File
@@ -788,13 +788,18 @@ function getXPHistory(req, res) {
═══════════════════════════════════════════════════════════════════════ */
/* POST /api/gamification/admin/award — award XP or coins to user */
const ADMIN_AWARD_MAX = 1_000_000;
function adminAward(req, res) {
const { userId, xp, coins, reason } = req.body;
if (!userId) return res.status(400).json({ error: 'userId required' });
const xpNum = Number(xp) || 0;
const coinsNum = Number(coins) || 0;
if (xpNum < 0 || xpNum > ADMIN_AWARD_MAX) return res.status(400).json({ error: `xp must be 0..${ADMIN_AWARD_MAX}` });
if (coinsNum < 0 || coinsNum > ADMIN_AWARD_MAX) return res.status(400).json({ error: `coins must be 0..${ADMIN_AWARD_MAX}` });
const user = stmts.checkUserById.get(userId);
if (!user) return res.status(404).json({ error: 'User not found' });
if (xp && xp > 0) awardXP(userId, xp, reason || 'Admin award');
if (coins && coins > 0) awardCoins(userId, coins, reason || 'Admin award');
if (xpNum > 0) awardXP(userId, xpNum, reason || 'Admin award');
if (coinsNum > 0) awardCoins(userId, coinsNum, reason || 'Admin award');
const updated = stmts.getUserGamInfo.get(userId);
res.json({ ok: true, ...updated });
}
backend/src/controllers/liveController.js
+23 -15
View File
@@ -36,7 +36,7 @@ function setQuestion(req, res) {
if (live.teacher_id !== teacher.id && teacher.role !== 'admin')
return res.status(403).json({ error: 'Forbidden' });
db.prepare('DELETE FROM live_answers WHERE live_session_id=?').run(liveId);
// Don't delete old answers — they're scoped per (session, user, question_id) so history is preserved
db.prepare(`UPDATE live_sessions SET question_id=?, status='active', show_results=0 WHERE id=?`)
.run(question_id, liveId);
@@ -67,9 +67,11 @@ function answer(req, res) {
).get(live.class_id, userId);
if (!isMember) return res.status(403).json({ error: 'Forbidden' });
if (!live.question_id) return res.status(400).json({ error: 'No active question' });
const existing = db.prepare(
'SELECT id FROM live_answers WHERE live_session_id=? AND user_id=?'
).get(liveId, userId);
'SELECT id FROM live_answers WHERE live_session_id=? AND user_id=? AND question_id=?'
).get(liveId, userId, live.question_id);
if (existing) return res.status(409).json({ error: 'Уже отвечено' });
let is_correct = null;
@@ -80,14 +82,20 @@ function answer(req, res) {
is_correct = opt ? opt.is_correct : 0;
}
db.prepare(
`INSERT INTO live_answers (live_session_id, user_id, option_id, answer_text, is_correct)
VALUES (?,?,?,?,?)`
).run(liveId, userId, option_id || null, answer_text || null, is_correct);
// Atomic INSERT — race-safe via UNIQUE(session, user, question_id)
try {
db.prepare(
`INSERT INTO live_answers (live_session_id, user_id, question_id, option_id, answer_text, is_correct)
VALUES (?,?,?,?,?,?)`
).run(liveId, userId, live.question_id, option_id || null, answer_text || null, is_correct);
} catch (e) {
if (String(e.message).includes('UNIQUE')) return res.status(409).json({ error: 'Уже отвечено' });
throw e;
}
const { c: count } = db.prepare(
'SELECT COUNT(*) AS c FROM live_answers WHERE live_session_id=?'
).get(liveId);
'SELECT COUNT(*) AS c FROM live_answers WHERE live_session_id=? AND question_id=?'
).get(liveId, live.question_id);
emit(live.teacher_id, { type: 'live_answer_count', liveId, count });
res.json({ ok: true, is_correct });
@@ -111,15 +119,15 @@ function results(req, res) {
SELECT o.id, o.text, o.is_correct,
COUNT(la.id) AS chosen_count
FROM options o
LEFT JOIN live_answers la ON la.option_id=o.id AND la.live_session_id=?
LEFT JOIN live_answers la ON la.option_id=o.id AND la.live_session_id=? AND la.question_id=?
WHERE o.question_id=?
GROUP BY o.id ORDER BY o.order_index
`).all(liveId, live.question_id);
`).all(liveId, live.question_id, live.question_id);
const stats = db.prepare(
`SELECT COUNT(*) AS total, SUM(is_correct) AS correct
FROM live_answers WHERE live_session_id=?`
).get(liveId);
FROM live_answers WHERE live_session_id=? AND question_id=?`
).get(liveId, live.question_id);
db.prepare('UPDATE live_sessions SET show_results=1 WHERE id=?').run(liveId);
emitToClass(live.class_id, { type: 'live_results', liveId, question, options, stats });
@@ -167,8 +175,8 @@ function getActive(req, res) {
'SELECT id, text, order_index FROM options WHERE question_id=? ORDER BY order_index'
).all(live.question_id);
myAnswer = db.prepare(
'SELECT option_id, is_correct FROM live_answers WHERE live_session_id=? AND user_id=?'
).get(live.id, req.user.id);
'SELECT option_id, is_correct FROM live_answers WHERE live_session_id=? AND user_id=? AND question_id=?'
).get(live.id, req.user.id, live.question_id);
}
res.json({ active: true, live, question, options, myAnswer, showResults: live.show_results });
backend/src/controllers/petController.js
+5 -12
View File
@@ -234,18 +234,11 @@ function buyBg(req, res) {
if (!user) return res.status(404).json({ error: 'not found' });
const owned = _parseOwned(user.pet_bg_owned);
if (!owned.includes(id)) {
if ((user.coins || 0) < item.price) return res.status(400).json({ error: 'insufficient_coins' });
const buy = db.transaction(() => {
// Re-check balance inside transaction to prevent race
const fresh = db.prepare('SELECT coins FROM users WHERE id=?').get(req.user.id);
if ((fresh.coins || 0) < item.price) throw new Error('insufficient_coins');
db.prepare('UPDATE users SET coins=coins-?, pet_bg_owned=?, pet_bg=? WHERE id=?')
.run(item.price, JSON.stringify([...owned, id]), id, req.user.id);
});
try { buy(); } catch (e) {
if (e.message === 'insufficient_coins') return res.status(400).json({ error: 'insufficient_coins' });
throw e;
}
// Atomic conditional UPDATE — race-safe even under concurrent purchase attempts
const result = db.prepare(
'UPDATE users SET coins = coins - ?, pet_bg_owned = ?, pet_bg = ? WHERE id = ? AND coins >= ?'
).run(item.price, JSON.stringify([...owned, id]), id, req.user.id, item.price);
if (result.changes === 0) return res.status(400).json({ error: 'insufficient_coins' });
} else {
db.prepare('UPDATE users SET pet_bg=? WHERE id=?').run(id, req.user.id);
}
backend/src/controllers/shopController.js
+11 -8
View File
@@ -30,14 +30,13 @@ function purchaseItem(req, res) {
const alreadyOwned = db.prepare('SELECT 1 FROM user_purchases WHERE user_id = ? AND item_id = ?').get(userId, itemId);
if (alreadyOwned) return res.status(400).json({ error: 'Вы уже купили этот предмет' });
// Atomic: check balance + deduct + insert purchase in one transaction
// Atomic: conditional UPDATE (race-safe even under concurrent purchases)
const doPurchase = db.transaction(() => {
const user = db.prepare('SELECT coins FROM users WHERE id = ?').get(userId);
if (!user || (user.coins || 0) < item.price) return { err: 'Недостаточно монет' };
db.prepare('UPDATE users SET coins = coins - ? WHERE id = ?').run(item.price, userId);
const upd = db.prepare(
'UPDATE users SET coins = coins - ? WHERE id = ? AND coins >= ?'
).run(item.price, userId, item.price);
if (upd.changes === 0) return { err: 'Недостаточно монет' };
db.prepare('INSERT INTO user_purchases (user_id, item_id) VALUES (?, ?)').run(userId, itemId);
const updated = db.prepare('SELECT coins FROM users WHERE id = ?').get(userId);
return { coins: (updated && updated.coins) || 0 };
});
@@ -166,10 +165,14 @@ function adminDeleteItem(req, res) {
}
/* POST /api/shop/admin/award-coins — award coins to user */
const SHOP_AWARD_MAX = 1_000_000;
function adminAwardCoins(req, res) {
const { userId, amount, reason } = req.body;
if (!userId || !amount || amount < 0) return res.status(400).json({ error: 'userId and positive amount required' });
db.prepare('UPDATE users SET coins = coins + ? WHERE id = ?').run(amount, userId);
const amt = Number(amount);
if (!userId || !Number.isFinite(amt) || amt <= 0 || amt > SHOP_AWARD_MAX)
return res.status(400).json({ error: `userId and amount (1..${SHOP_AWARD_MAX}) required` });
const result = db.prepare('UPDATE users SET coins = coins + ? WHERE id = ?').run(amt, userId);
if (result.changes === 0) return res.status(404).json({ error: 'User not found' });
const user = db.prepare('SELECT coins FROM users WHERE id = ?').get(userId);
res.json({ ok: true, coins: user?.coins || 0 });
}