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
Фикс: 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 + <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 || 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 | Остальное (см. таблицу 🟡) | Качество |