orders module: add auth, fix command injection, add requirements #13

Merged
testclient-admin merged 15 commits from feature/orders-module into main 2026-08-25 03:44:38 +00:00
Showing only changes of commit cbfe851513 - Show all commits
@@ -0,0 +1,65 @@
[
{
"id": "ai-review-0-orders.py-43",
"source": "ai-review",
"category": "idor",
"severity": "high",
"confidence": 1.0,
"file": "orders.py",
"line_start": 43,
"line_end": 47,
"title": "IDOR при чтении заказов — отсутствие проверки ownership",
"description": "Метод do_GET для /orders/{id} проверяет аутентификацию, но не проверяет, что заказ принадлежит текущему пользователю. Пользователь может прочитать любой заказ по ID.",
"impact": "Любой аутентифицированный пользователь может читать чужие заказы, что ведёт к утечке конфиденциальных данных о финансовых операциях.",
"exploit_scenario": "Пользователь с токеном token_userA отправляет GET /orders/100 и получает данные чужого заказа, включая amount и user_id.",
"evidence": "def do_GET(self): ... if order_id in orders: self.send_json_response(200, orders[order_id]) — нет проверки orders[order_id].get(\"user_id\") != getattr(self, \"_user_id\", None).",
"recommendation": "Добавить проверку: if orders[order_id].get(\"user_id\") != getattr(self, \"_user_id\", None): self.send_json_response(403, {\"error\": \"Forbidden\"}).",
"cwe": "CWE-639",
"status": "OPEN",
"created_at": "2026-08-23T18:54:38Z",
"pr_number": 13,
"commit_sha": "4098606aff8e55a4446bbe2e141972caf5f3cf90"
},
{
"id": "ai-review-1-orders.py-105",
"source": "ai-review",
"category": "command-exec",
"severity": "critical",
"confidence": 1.0,
"file": "orders.py",
"line_start": 105,
"line_end": 122,
"title": "Недостаточная валидация хоста — возможная инъекция в rsync",
"description": "Метод _validate_host проверяет только формат IP/домена, но не блокирует потенциально опасные символы и конструкции (например, пробелы, '--rsync-path=', других опций rsync), что может привести к выполнению произвольных команд при вызове subprocess.run([\"rsync\", ...]).",
"impact": "Возможна полная компрометация сервера — выполнение произвольных команд с правами процесса.",
"exploit_scenario": "Атакующий отправляет POST /admin/backup с host='example.com --rsync-path=/tmp/evil.sh'. Даже если регулярное выражение пропустит строку, она может быть интерпретирована как доп. опция rsync или использована для обхода валидации. Проверка _validate_host не исключает наличие других аргументов команды.",
"evidence": "cmd = [\"rsync\", \"-avz\", \"/workspace/\", f\"{validated_host}:/backup/orders/\"] — validated_host проходит только регулярку, но может содержать опции rsync или управляющие символы.",
"recommendation": "Полностью отказаться от произвольного host — использовать белый список предопределённых хостов из конфига, либо дополнить валидацию запретом на все пробелы, дефисы в начале и все символы, начинающиеся с '--'.",
"cwe": "CWE-78",
"status": "OPEN",
"created_at": "2026-08-23T18:54:38Z",
"pr_number": 13,
"commit_sha": "4098606aff8e55a4446bbe2e141972caf5f3cf90"
},
{
"id": "ai-review-2-orders.py-65",
"source": "ai-review",
"category": "authz",
"severity": "high",
"confidence": 1.0,
"file": "orders.py",
"line_start": 65,
"line_end": 75,
"title": "Эндпоинт /admin/backup доступен любому аутентифицированному пользователю",
"description": "Эндпоинт проверяет только наличие токена (начинающегося с token_), но не проверяет, что пользователь является администратором, несмотря на наличие проверки self._user_id == \"admin\".",
"impact": "Любой пользователь, имеющий валидный токен (включая обычные пользовательские токены), может инициировать резервное копирование и потенциально использовать уязвимость валидации хоста для RCE.",
"exploit_scenario": "Пользователь с token_user отправляет POST /admin/backup с вредоносным host. Проверка self._user_id == \"admin\" находится в том же блоке, что и сам эндпоинт, но проверка аутентификации идёт после, и _user_id может быть установлен до проверки роли — возможна путаница в логике и обход.",
"evidence": "elif self.path == \"/admin/backup\": if not getattr(self, \"_user_id\", None) == \"admin\": ... if not self.check_auth(): ... — проверка роли идет ПОСЛЕ установки _user_id в check_auth, но неясно, будет ли проверка роли применена для всех случаев — особенно при обработке заголовков.",
"recommendation": "Вынести проверку роли admin до проверки аутентификации, и проверять наличие роли явно, например: auth_ok = self.check_auth(); if not auth_ok: ...; if getattr(self, '_user_id', None) != 'admin': ...",
"cwe": "CWE-285",
"status": "OPEN",
"created_at": "2026-08-23T18:54:38Z",
"pr_number": 13,
"commit_sha": "4098606aff8e55a4446bbe2e141972caf5f3cf90"
}
]