Files
ipwhitelist-app/research/opus-review-ejs.md
T

7.8 KiB
Raw Blame History

Код-ревью index.ejs — XSS, CSRF, clickjacking, client-валидация

Дата: 2026-05-30

🟢 XSS — экранирование корректно

Весь динамический вывод идёт через <%= %>, который EJS экранирует (&<>"'). <%- %> не используется нигде. Векторы проверены:

  • <%= message %>, <%= error %> — экранируются. Даже если в error попадёт сырая ошибка БД с <script>, она будет обезврежена.
  • <%= e.value_cidr %>, <%= e.comment %>, <%= e.created_by %> (данные из БД) — экранируются.
  • <%= user.clientId %>, <%= user.email %> — экранируются.

Сохранённого XSS через комментарий/email нет. Это сильная сторона шаблона.

⚠️ Единственный нюанс: action="/delete/<%= e.id %>"e.id идёт в атрибут URL. Так как это integer из БД (SERIAL), инъекция невозможна. Но если тип когда-нибудь станет строковым — атрибутный контекст потребует особой осторожности. Сейчас безопасно.

🔴 CSRF — формы без токена (критично)

<form method="POST" action="/add">
<form method="POST" action="/delete/<%= e.id %>" ...>

Ни одна форма не содержит CSRF-токена. Обе меняют состояние. Сторонний сайт может авто-сабмитить POST на /add//delete/:id. Зеркалит находку из ревью server.js. Фикс — пробросить токен из middleware (csurf) и в каждой форме:

<form method="POST" action="/add">
  <input type="hidden" name="_csrf" value="<%= csrfToken %>">
  ...
</form>
<form method="POST" action="/delete/<%= e.id %>" style="display:inline" onsubmit="return confirm('Удалить запись?')">
  <input type="hidden" name="_csrf" value="<%= csrfToken %>">
  <button class="btn btn-danger">Удалить</button>
</form>

(Требует прокидывания csrfToken в res.render во всех роутах server.js.)

🔴 Clickjacking — нет защиты фрейминга

Шаблон с кнопками «Удалить» можно встроить в <iframe> на фишинговом сайте и подложить под клик (UI redress). В самом EJS защиты нет — нужны заголовки на стороне server.js (helmetX-Frame-Options: DENY / CSP frame-ancestors 'none'). Дублирует находку из ревью server.js. На уровне шаблона можно добавить CSP через meta (слабее заголовка, но лучше чем ничего):

<meta http-equiv="Content-Security-Policy" content="frame-ancestors 'none'; default-src 'self'; style-src 'self' 'unsafe-inline'">

⚠️ style-src 'unsafe-inline' потребуется из-за инлайн-<style> и inline-атрибутов style="..." в header/кнопках — это ослабляет CSP. По-хорошему вынести стили в отдельный .css файл и убрать unsafe-inline.

🟡 disabled-поля обходятся через DevTools (это и есть главная дыра валидации)

<input name="value" ... <%= used >= limit ? 'disabled' : '' %>>
<button ... <%= used >= limit ? 'disabled' : '' %>>

disabled — только UX. Атакующий через DevTools снимает атрибут и шлёт POST /add сверх лимита. ЭТО НЕ УЯЗВИМОСТЬ ШАБЛОНА, пока сервер проверяет лимит — а он проверяет (createEntry). Вывод: клиентский disabled не является защитой и не должен ею считаться; настоящая защита — серверная проверка лимита (она есть, но уязвима к гонке — см. ревью queries.js). Шаблон тут корректен ровно при условии серверной проверки.

🟡 Нет client-side валидации (несоответствие ТЗ)

ТЗ требует клиентскую валидацию формата IPv4/CIDR. Сейчас только required и серверная проверка. Пользователь узнаёт об ошибке только после round-trip. Добавить pattern для базовой проверки + JS для маски /22–/32:

<input name="value"
       pattern="^(\d{1,3}\.){3}\d{1,3}(/\d{1,2})?$"
       title="IPv4 или CIDR, например 203.0.113.0/24"
       placeholder="Например: 203.0.113.10 или 203.0.113.0/24"
       required <%= used >= limit ? 'disabled' : '' %>>

pattern — только формат; диапазон маски (/22–/32) и host-биты всё равно валидирует сервер (validators.js). Это UX-улучшение, не замена серверной проверки.

🟡 maxlength только на comment, не на value

comment имеет maxlength="255" (совпадает со схемой VARCHAR(255) — хорошо). У value нет maxlength — стоит добавить maxlength="18" под VARCHAR(18), чтобы не слать заведомо длинное и для согласованности.

🟢 Утечка чужих данных — нет

В шаблоне выводятся только user.clientId/user.email (свои) и entries (своей компании, отфильтрованы по company_id в listEntries). Данных других компаний нет. Изоляция на уровне шаблона соблюдена (зависит от корректной фильтрации в queries.js — там она есть).

🟡 onsubmit confirm — не защита, но ок

onsubmit="return confirm(...)" легко обходится, но это UX-подтверждение, не security-контроль. Приемлемо.

🟡 favicon/иконка — внешних ресурсов нет

Все ресурсы локальные (/favicon.png, инлайн SVG, инлайн CSS). Нет внешних CDN → меньше поверхность для supply-chain. Хорошо. Обратная сторона — инлайн-стили мешают строгой CSP (см. выше).


Итог:

  1. 🟢 XSS нет — всё через <%= %>, <%- %> не используется. Главная сильная сторона.
  2. 🔴 CSRF-токенов в формах нет — добавить _csrf в /add и /delete (+ middleware в server.js).
  3. 🔴 Clickjacking — защита только заголовками (helmet в server.js); опционально CSP-meta.
  4. 🟡 disabled по лимиту обходится через DevTools — не баг шаблона при условии серверной проверки (она есть).
  5. 🟡 Нет client-валидации формата (ТЗ требует) — добавить pattern + maxlength на value.
  6. 🟡 Инлайн-стили вынудят unsafe-inline в CSP — вынести в отдельный .css для строгой политики.

Шаблон по XSS написан правильно; основные пробелы — CSRF и clickjacking (закрываются в server.js) и отсутствие клиентской валидации из ТЗ.