Files
ipwhitelist-app/research/REVIEW-SUMMARY.md
T

8.1 KiB

Сводка код-ревью IP WhiteList (все 5 файлов)

Дата: 2026-05-30

🔴 Критические (блокеры прода)

1. JWT без проверки подписи — server.js

const payload = JSON.parse(Buffer.from(auth.replace('Bearer ', '').split('.')[1], 'base64').toString());

Декодирует payload без верификации подписи. Любой подделывает токен → любая компания. Нужна jose.jwtVerify(token, JWKS, { issuer: 'auth-api' }).

2. Пустой req.user не возвращает 401 — server.js

} catch { req.user = {}; }
next();

Невалидный токен → req.user = {}clientId = undefined → анонимы делят NULL-компанию. Нужно if (!req.user.clientId) return res.status(401).send(...).

3. Race conditions (TOCTOU) — queries.js

Три гонки из-за отсутствия транзакций:

  • Обход лимита: два параллельных запроса читают cnt=14, оба вставляют → 16 записей
  • Дубли/пересечения: проверка existing и INSERT не в транзакции
  • Дубли компаний: getOrCreateCompany — два первых запроса новой компании оба не находят, оба INSERT

Фикс: BEGINSELECT ... FOR UPDATE строки компании → проверки → INSERT/UPDATE → logAudit(..., client)COMMIT.

4. audit вне транзакции — queries.js

logAudit использует глобальный pool, не клиент транзакции. При сбое: запись есть, аудита нет (или наоборот). Передавать клиент транзакции в logAudit.

5. /export без авторизации — server.js

getExportCIDRs() без аргументов отдаёт CIDR всех компаний без проверки req.user. Публичная утечка whitelist всех клиентов.

6. DEV_MODE не привязан к NODE_ENV — server.js

DEV_MODE=true в проде → все становятся WZ01325 без auth. Добавить && process.env.NODE_ENV !== 'production'.

7. CSRF — server.js + index.ejs

POST-формы /add, /delete/:id без CSRF-токенов. Нужен csurf + <input name="_csrf"> в формах.

8. Обход запрещённых диапазонов через суперсеть — validators.js

Блокировка использует isSubnetOf(normalized, blocked) — можно обойти /22, содержащей запрещённый /24 (например 192.0.2.0/22 содержит TEST-NET-1). Заменить на overlaps(normalized, blocked).


🟠 Схема БД (structure)

9. Нет UNIQUE на активный CIDR компании — schema.sql

Дубли держатся только на коде. Добавить:

CREATE UNIQUE INDEX uq_entries_active_cidr
    ON whitelist_entries(company_id, value_cidr) WHERE deleted_at IS NULL;

10. value_cidr как VARCHAR — нет проверок в БД — schema.sql

БД не валидирует формат и не ловит пересечения. Варианты:

Уровень Что
Минимум CHECK (value_cidr ~ '^(\d{1,3}\.){3}\d{1,3}/\d{1,2}$')
Production Тип CIDR + exclusion constraint (btree_gist) для пересечений

11. FK без явного ON DELETE — schema.sql

REFERENCES companies(id) без указания поведения. Явно задать ON DELETE RESTRICT.


🟡 Средние (production-hardening)

# Где Что
12 server.js Нет helmet — X-Frame-Options, CSP, HSTS отсутствуют
13 server.js Нет rate-limit на /add, /delete, /export
14 server.js express.urlencoded без limit — DoS большими телами
15 server.js Нет глобального error-handler middleware
16 server.js /healthz под auth — сломает k8s-пробу. Вынести выше
17 server.js ?error= в редиректе не читается в res.render('/', ...) — параметр молча теряется
18 server.js Сырые e.message БД наружу — info leak. Маппить на дружелюбные сообщения
19 queries.js custom_limit = 0 игнорируется: company.custom_limit || defaultLimit0 || 15 = 15
20 queries.js getOrCreateCompany не атомарен (хотя UNIQUE на client_id спасает). Upsert: INSERT ... ON CONFLICT
21 queries.js deleteEntry WHERE только по id — добавить AND company_id = $3 для глубины защиты
22 schema.sql audit_log.company_id без FK на companies (допустимо, но задокументировать)
23 schema.sql audit_log.action — свободный VARCHAR. Добавить CHECK (action IN ('CREATE','UPDATE','DELETE'))
24 schema.sql Индекс аудита только на company_id. Добавить (company_id, created_at DESC)
25 schema.sql updated_at не обновляется автоматически. Добавить триггер
26 schema.sql SERIALGENERATED ALWAYS AS IDENTITY (PG 10+)
27 schema.sql custom_limit без CHECK ≥ 0
28 validators.js parseInt('24abc') = 24 — глотает мусор. Проверять ^\d{1,2}$
29 validators.js Множественные слэши не отсекаются: 10.0.0.0/24/8 → средняя часть игнорируется
30 index.ejs Нет client-валидации формата (ТЗ требует). Добавить pattern + maxlength="18"
31 index.ejs disabled по лимиту обходится через DevTools — не баг, т.к. сервер проверяет
32 index.ejs Инлайн-стили → unsafe-inline в CSP. Вынести в .css для строгой политики

🟢 Безопасно (проверено)

  • SQL-инъекций нет — все запросы параметризованы ($1, $2...)
  • XSS в EJS нет — всё через <%= %>, <%- %> не используется
  • Изоляция компаний корректнаupdateEntry/deleteEntry проверяют company_id, listEntries фильтрует по компании
  • Сохранённого XSS через БД нет — все поля экранируются
  • overlaps() формула корректна — проверено 12 тестами, старая и новая формулы математически эквивалентны
  • Изоляция через схему БД — записи привязаны к company_id, обход только через код (не схему)
  • Partial-индекс idx_entries_active — правильный приём, soft-deleted не раздувают индекс

Приоритет исправлений

Порядок Что Блокирует
1 JWT — проверка подписи Продакшен
2 Транзакции в createEntry/updateEntry/deleteEntry Целостность данных
3 UNIQUE на активный CIDR в БД Защита от гонок
4 401 при пустом req.user Auth
5 DEV_MODE → NODE_ENV Безопасность прода
6 /export — авторизация Утечка данных
7 CSRF-токены Безопасность
8 overlaps вместо isSubnetOf в блокировке Валидация
9 helmet + rate-limit Production-hardening
10 Остальное (см. таблицу 🟡) Качество