v0.0.73: фиксы по код-ревью Соннета — SSRF, path traversal, слабый SID, proc_error, zip-бомба, self-XSS
Deploy drhider / validate (push) Canceled after 0s
Deploy drhider / validate (push) Canceled after 0s
This commit is contained in:
@@ -106,6 +106,7 @@
|
||||
- **2026-08-24-time-tickers-estimates.md** — v0.0.64–0.0.65: тикер текущего файла на всех этапах, мгновенные оценки времени по файлам и суммарно.
|
||||
- **2026-08-24-folder-select-implemented.md** — v0.0.68: кнопка «Выбрать папку» (webkitdirectory, рекурсивно, относительный путь).
|
||||
- **2026-08-24-folder-zip-not-lost.md** — v0.0.69: архивы из папки без документов не теряются (добавляются как есть); раскрытие zip с документами; склонение счётчика.
|
||||
- **2026-08-24-v073-security-fixes.md** — v0.0.73: 6 фиксов по ревью Соннета (SSRF, path traversal, слабый SID, proc_error, zip-бомба, self-XSS); 1 отклонено (cleanup), 5 отложено.
|
||||
- **2026-08-24-pull-retry-limit-counter.md** — v0.0.72: ретраи pull из ВМ, корректная ошибка лимита сессии, счётчик дедупа.
|
||||
- **2026-08-24-code-review-sonnet.md** — код-ревью Соннета: 15 багов (SSRF, path traversal, слабый SID, zip-бомба и др.), промпт в docs/code-review-sonnet.md.
|
||||
- **2026-08-24-upload-logic-schema.md** — подробная схема логики загрузки + найденные баги/несоответствия.
|
||||
|
||||
@@ -0,0 +1,39 @@
|
||||
# v0.0.73 — фиксы по код-ревью Соннета (безопасность + явные баги) (2026-08-24)
|
||||
|
||||
_code-fixes. По ревью `History/sonnet/2026-08-24-code-review-sonnet.md` (промпт `docs/code-review-sonnet.md`)._
|
||||
_Критично перепроверены в коде; спорные пункты — отклонены/отложены (см. ниже)._
|
||||
|
||||
## Исправлено (6 фиксов)
|
||||
|
||||
1. **SSRF** (`api_bp.py::upload_refs`) — добавлена валидация `url.startswith(VM_UPLOAD_PREFIX)`;
|
||||
недоверенный URL пропускается. Константа `VM_UPLOAD_PREFIX = "https://contracts.kube5s.ru/drhider-upload/"`.
|
||||
2. **Path traversal / zip slip** (`api_bp.py`) — функция `_safe_name()`: нормализует слэши,
|
||||
отбрасывает `..` и абсолютные пути, сохраняя подпапки (`Подпапка/Акт.txt` → как есть,
|
||||
`../../evil.pdf` → ""). Применена в `upload` и `upload_refs`.
|
||||
Unit-тест 8 кейсов — все OK.
|
||||
3. **Слабый SID** (`session.py`) — `uuid.uuid4().hex[:12]` → `uuid.uuid4().hex` (128 бит).
|
||||
4. **Серверная ошибка не отображалась** — серверное событие `event: error` (конфликт с встроенным
|
||||
EventSource) → `event: proc_error`; добавлен клиентский `addEventListener('proc_error', …)`
|
||||
с показом сообщения.
|
||||
5. **ZIP-бомба: обход через поддельный `file_size`** (`extractor.py`) — добавлена проверка
|
||||
`total_uncompressed > MAX_UNCOMPRESSED` ПОСЛЕ `zf.read()` с `break` (ранний выход).
|
||||
6. **Self-XSS через имя файла** (`index.html`) — добавлена `esc()` и применена к `f.name`
|
||||
в `rr()` и `procRow()`.
|
||||
|
||||
## Отклонено (критично к Соннету)
|
||||
- **«cleanup(sid) после /download»** — НЕ сделано: ZIP и CSV скачиваются РАЗДЕЛЬНЫМИ запросами;
|
||||
удаление сессии после отдачи ZIP сломало бы скачивание CSV. Сессия и так чистится по TTL (30 мин).
|
||||
|
||||
## Отложено (требуют решения/риск)
|
||||
- idx-мисматч при `expand_zips` (фильтр расширений на бэке) — связано с фичей v0.0.69
|
||||
(zip без документов «как есть»), требует решения по поведению.
|
||||
- Отмена не проверяется в extract-фазе (.doc liberta 120с) — отдельный фикс.
|
||||
- LLM-таймаут тихо обнуляет чанк — логирование/статус.
|
||||
- Debug-эндпоинт `session_files` — ограничить/закрыть.
|
||||
- LLM prompt injection — задокументировать (класс риска, не фикс кода).
|
||||
|
||||
## Проверка
|
||||
- `py_compile` (app.py, api_bp.py, session.py, extractor.py) — OK; `node --check` — OK.
|
||||
- `_safe_name` unit-тест — 8/8 OK.
|
||||
- `create_app()` стартует (VERSION 0.0.73).
|
||||
- Версия 0.0.72 → 0.0.73.
|
||||
@@ -339,6 +339,9 @@ def expand_zips(files: List[Tuple[str, bytes, str]]) -> List[Tuple[str, bytes, s
|
||||
|
||||
inner_data = zf.read(info)
|
||||
total_uncompressed += len(inner_data)
|
||||
if total_uncompressed > MAX_UNCOMPRESSED:
|
||||
log.warning("ZIP uncompressed limit exceeded (mid-read): %s", fname)
|
||||
break
|
||||
|
||||
# Вложенные ZIP добавляем в очередь на повторную распаковку
|
||||
queue.append((name, inner_data, ""))
|
||||
|
||||
+1
-1
@@ -21,7 +21,7 @@ if _sys_path_root not in sys.path:
|
||||
sys.path.insert(0, _sys_path_root)
|
||||
|
||||
# Версия приложения (меняется при изменениях)
|
||||
VERSION = "0.0.72"
|
||||
VERSION = "0.0.73"
|
||||
|
||||
|
||||
def setup_logging():
|
||||
|
||||
+29
-6
@@ -34,6 +34,24 @@ log = logging.getLogger("routes.api_bp")
|
||||
PULL_RETRIES = 3
|
||||
PULL_RETRY_DELAY = 2 # секунды между попытками
|
||||
|
||||
# Доверенный префикс ВМ-буфера — валидация URL при pull (защита от SSRF)
|
||||
VM_UPLOAD_PREFIX = "https://contracts.kube5s.ru/drhider-upload/"
|
||||
|
||||
|
||||
def _safe_name(name: str) -> str:
|
||||
"""Санитизировать имя файла: защита от path traversal, сохраняя подпапки.
|
||||
|
||||
Запрещает '..' и абсолютные пути; нормализует слэши. Возвращает "" если
|
||||
имя пустое или небезопасное.
|
||||
"""
|
||||
if not name:
|
||||
return ""
|
||||
name = name.replace("\\", "/")
|
||||
parts = [p for p in name.split("/") if p and p != "."]
|
||||
if not parts or any(p == ".." for p in parts):
|
||||
return ""
|
||||
return "/".join(parts)
|
||||
|
||||
|
||||
def _disconnect_exceptions():
|
||||
"""Исключения, означающие отключение клиента SSE."""
|
||||
@@ -54,16 +72,17 @@ def upload():
|
||||
added = 0
|
||||
had_unnamed = False
|
||||
for f in uploaded:
|
||||
if not f.filename:
|
||||
name = _safe_name(f.filename)
|
||||
if not name:
|
||||
had_unnamed = True
|
||||
continue
|
||||
data = f.read()
|
||||
log.info("upload: sid=%s file=%r size=%d", sid, f.filename, len(data))
|
||||
log.info("upload: sid=%s file=%r size=%d", sid, name, len(data))
|
||||
if len(data) > MAX_FILE_BYTES:
|
||||
log.warning("upload: file exceeds %dMB, skipped sid=%s file=%r size=%d",
|
||||
MAX_FILE_BYTES // (1024 * 1024), sid, f.filename, len(data))
|
||||
MAX_FILE_BYTES // (1024 * 1024), sid, name, len(data))
|
||||
continue
|
||||
if not add_file(sid, f.filename, data):
|
||||
if not add_file(sid, name, data):
|
||||
log.warning("upload: session not found/limit, sid=%s file=%r", sid, f.filename)
|
||||
return jsonify({"ok": False, "error": "Session not found"}), 404
|
||||
added += 1
|
||||
@@ -96,10 +115,14 @@ def upload_refs():
|
||||
try:
|
||||
with httpx.Client(timeout=120, follow_redirects=True) as client:
|
||||
for ref in refs:
|
||||
name = ref.get("name")
|
||||
name = _safe_name(ref.get("name") or "")
|
||||
url = ref.get("url")
|
||||
if not name or not url:
|
||||
continue
|
||||
# SSRF-защита: тянуть можно ТОЛЬКО с доверенного ВМ-буфера
|
||||
if not url.startswith(VM_UPLOAD_PREFIX):
|
||||
log.warning("upload_refs: unsafe URL, skip sid=%s url=%r", sid, url)
|
||||
continue
|
||||
# Лимит на один файл (50 МБ): сверх лимита — пропускаем (не участвует)
|
||||
if (ref.get("size") or 0) > MAX_FILE_BYTES:
|
||||
log.warning("upload_refs: file exceeds %dMB, skip sid=%s file=%r size=%s",
|
||||
@@ -381,7 +404,7 @@ def process_stream(sid):
|
||||
_, msg = evt
|
||||
log.error("process_stream: error event sid=%s msg=%r", sid, msg)
|
||||
try:
|
||||
yield f"event: error\ndata: {json.dumps({'error': msg})}\n\n"
|
||||
yield f"event: proc_error\ndata: {json.dumps({'error': msg})}\n\n"
|
||||
except _disconnect_exceptions() as e:
|
||||
log.warning("process_stream: disconnect on error sid=%s err=%r", sid, e)
|
||||
return
|
||||
|
||||
+1
-1
@@ -104,7 +104,7 @@ def create_session() -> str:
|
||||
Returns:
|
||||
Уникальный идентификатор сессии (UUID).
|
||||
"""
|
||||
sid = uuid.uuid4().hex[:12]
|
||||
sid = uuid.uuid4().hex
|
||||
with _lock:
|
||||
_sessions[sid] = {
|
||||
"files": [],
|
||||
|
||||
@@ -280,6 +280,7 @@ function fmtSec(s) {
|
||||
// Эмпирическая оценка времени обработки файла: сек/МБ (ориентировочно, до старта)
|
||||
const EST_MB_SEC = 12;
|
||||
function estForFile(f) { return f ? Math.max(1, Math.round(f.size / 1048576 * EST_MB_SEC)) : 0; }
|
||||
function esc(s) { return String(s).replace(/[&<>"']/g, c => ({'&':'&','<':'<','>':'>','"':'"',"'":'''}[c])); }
|
||||
|
||||
function rr() {
|
||||
if (procPhase === 'processing') { renderProcTable(); return; }
|
||||
@@ -290,7 +291,7 @@ function rr() {
|
||||
const rowCls = over ? ' class="row-over"' : '';
|
||||
const stTxt = over ? '<span style="color:#c0392b;">🔥 не учитывается</span>'
|
||||
: '<span style="color:#7d3c98;">~' + fmtSec(estForFile(f)) + '</span>';
|
||||
return '<tr id="row-' + i + '"' + rowCls + '><td class="name-cell">' + f.name + '</td><td class="num-cell">' + fs(f.size) + '</td><td class="num-cell" id="st-' + i + '" style="font-size:12px;">' + stTxt + '</td><td><button class="remove-btn" onclick="rm(' + i + ')">✕</button></td></tr>';
|
||||
return '<tr id="row-' + i + '"' + rowCls + '><td class="name-cell">' + esc(f.name) + '</td><td class="num-cell">' + fs(f.size) + '</td><td class="num-cell" id="st-' + i + '" style="font-size:12px;">' + stTxt + '</td><td><button class="remove-btn" onclick="rm(' + i + ')">✕</button></td></tr>';
|
||||
}).join('');
|
||||
}
|
||||
const overCount = sf.filter(f => overNames.has(f.name)).length;
|
||||
@@ -307,7 +308,7 @@ function procRow(i, stTxt) {
|
||||
const over = overNames.has(f.name);
|
||||
const cls = (procState[i] && procState[i].st === 'current') ? ' class="row-current"'
|
||||
: (over ? ' class="row-over"' : '');
|
||||
return '<tr' + cls + '><td class="name-cell">' + f.name + '</td><td class="num-cell">' + fs(f.size) + '</td><td class="num-cell" style="font-size:12px;">' + stTxt + '</td><td></td></tr>';
|
||||
return '<tr' + cls + '><td class="name-cell">' + esc(f.name) + '</td><td class="num-cell">' + fs(f.size) + '</td><td class="num-cell" style="font-size:12px;">' + stTxt + '</td><td></td></tr>';
|
||||
}
|
||||
|
||||
function renderProcTable() {
|
||||
@@ -936,6 +937,14 @@ async function uploadFiles() {
|
||||
document.getElementById('newSessionBtn').style.display = 'inline-block';
|
||||
resolve();
|
||||
});
|
||||
activeES.addEventListener('proc_error', function(e) {
|
||||
const d = JSON.parse(e.data);
|
||||
finishProcUI();
|
||||
setBusy(false);
|
||||
st.className = 'status error';
|
||||
st.textContent = 'Ошибка обработки: ' + (d.error || 'неизвестная ошибка');
|
||||
resolve();
|
||||
});
|
||||
activeES.onerror = function() {
|
||||
finishProcUI();
|
||||
reject(new Error('SSE connection failed'));
|
||||
|
||||
Reference in New Issue
Block a user