T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/generate_3d_model.py:170
- Finding
- API-Controlled Path Components Allow Arbitrary File Writes## Vulnerability Details **File Location**: `scripts/generate_3d_model.py`, lines 170-176 and 184-208 **Vulnerability Type**: Path traversal and unrestricted file write **Risk Level**: High ### Vulnerable Code ```python # If output directory is specified, download model if args.output: # Create task-specific output directory task_output_dir = os.path.join(args.output, task_uuid) print(f"\nDownloading models to: {task_output_dir}") for file_info in file_list: download_model(file_info.get('url'), task_output_dir, file_info.get('name')) ``` ```python def download_model(model_url, output_dir, filename=None): """ Download 3D model to specified directory Args: model_url: Model download link output_dir: Output directory filename: Filename (optional) """ import requests # Ensure output directory exists os.makedirs(output_dir, exist_ok=True) # Get filename if not filename: filename = os.path.basename(model_url.split("?")[0]) output_path = os.path.join(output_dir, filename) # Download file try: response = requests.get(model_url, stream=True, timeout=60) response.raise_for_status() with open(output_path, "wb") as f: for chunk in response.iter_content(chunk_size=8192): if chunk: f.write(chunk) ``` ### Technical Analysis The `task_uuid` and `file_info["name"]` values originate from the remote API response. They are passed to `os.path.join()` without validation or canonical containment checks. `os.path.join()` does not enforce confinement beneath its first argument. A filename containing traversal components such as `../../target` can escape the output directory. On supported platforms, an absolute second path can also cause the intended base path to be discarded. The API-controlled `task_uuid` creates an additi ...[truncated 1313 chars]
- Remediation
- ## Remediation Suggestions - Do not use API-provided identifiers directly as local path components. Generate local filenames and task-directory names using trusted UUID generation. - Reduce API-provided filenames to a safe basename and reject absolute paths, parent-directory components, path separators, empty names, and platform-specific alternate separators. - Resolve both the output root and candidate destination with `pathlib.Path.resolve()`, then verify that the destination is a descendant of the output root before opening it. - Open newly created files with exclusive creation where overwriting is unnecessary. - Maintain an explicit allowlist of expected filename extensions. - Apply restrictive permissions to output directories and files. - A suitable containment pattern is: ```python from pathlib import Path import uuid root = Path(args.output).resolve() task_dir = (root / str(uuid.uuid4())).resolve() task_dir.mkdir(parents=True, exist_ok=True) supplied_name = Path(filename).name if supplied_name != filename or supplied_name in {"", ".", ".."}: raise ValueError("Unsafe filename") destination = (task_dir / supplied_name).resolve() if task_dir not in destination.parents: raise ValueError("Destination escapes output directory") ```