98 lines
7.8 KiB
Markdown
98 lines
7.8 KiB
Markdown
# Код-ревью 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 — формы без токена (критично)
|
||
|
||
```html
|
||
<form method="POST" action="/add">
|
||
<form method="POST" action="/delete/<%= e.id %>" ...>
|
||
```
|
||
|
||
Ни одна форма не содержит CSRF-токена. Обе меняют состояние. Сторонний сайт может авто-сабмитить POST на `/add`/`/delete/:id`. Зеркалит находку из ревью server.js. Фикс — пробросить токен из middleware (`csurf`) и в каждой форме:
|
||
|
||
```html
|
||
<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 (`helmet` → `X-Frame-Options: DENY` / CSP `frame-ancestors 'none'`). Дублирует находку из ревью server.js. На уровне шаблона можно добавить CSP через meta (слабее заголовка, но лучше чем ничего):
|
||
|
||
```html
|
||
<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 (это и есть главная дыра валидации)
|
||
|
||
```html
|
||
<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:
|
||
|
||
```html
|
||
<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) и отсутствие клиентской валидации из ТЗ.
|