# Сводка код-ревью IP WhiteList (все 5 файлов) > Дата: 2026-05-30 ## 🔴 Критические (блокеры прода) ### 1. JWT без проверки подписи — server.js ```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 ```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 **Фикс:** `BEGIN` → `SELECT ... 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` + `` в формах. ### 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 Дубли держатся только на коде. Добавить: ```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 \|\| defaultLimit` → `0 \|\| 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 | `SERIAL` → `GENERATED 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 | Остальное (см. таблицу 🟡) | Качество |