T09 · Insecure Skill Coding Practices
Error
- Location
src/ocr2markdown.py:58- Finding
Unsanitized Slug Allows Path Traversal Outside the Intended Task Directory
- Content
View full analysis
list[dict]: _bootstrap_cache() upload_dir = Path(config.MOUNT_DATA) / slug / config.DIR_UPLOAD output_base = Path(config.MOUNT_DATA) / slug / config.DIR_OUTPUT ``` The `slug` value originates from the local command-line entry point and is passed directly to the remote function: ```python @images.app.local_entrypoint() def main(slug: str) -> None: print(f"[ocr2markdown] processing: {slug}") results = ocr2markdown.remote(slug) ``` ### Technical Analysis The application treats `slug` as a safe directory name without validating or normalizing it. Python's `pathlib` permits both parent-directory components and absolute paths: - A slug containing `..` can escape the intended task directory beneath `/mnt/data`. - If `slug` is absolute, the preceding `config.MOUNT_DATA` component is discarded. - The resulting paths are subsequently used to discover input PDFs, create output directories, and pass paths to MinerU. The subprocess call does not create shell command injection because it uses an argument list rather than `shell=True`. However, the attacker-controlled path still determines which PDF files MinerU reads and where generated files are written. The affected Modal function mounts both the shared data volume at `/mnt/data` and the shared model volume at `/mnt/models`. Consequently, traversal can cross task boundaries and potentially reach other writable mounted or container locations. ### Attack Path 1. An authenticated caller invokes the skill with a crafted slug, for example: ```bash modal run ./src/ocr2markdown.py --slug ../models ``` 2. The application constructs these effective paths: ```text /mnt/data/../models/upload /mnt/data/../models/output ...[truncated 1136 chars]- Remediation
View remediation
str: if not SLUG_PATTERN.fullmatch(slug): raise ValueError("Invalid slug") return slug ``` 2. Resolve constructed paths and verify that they remain beneath the intended base directory: ```python def safe_child(base: Path, slug: str, child: str) -> Path: base = base.resolve() candidate = (base / validate_slug(slug) / child).resolve() if not candidate.is_relative_to(base): raise ValueError("Path escapes the data directory") return candidate ``` 3. Construct both directories through the validated helper: ```python data_root = Path(config.MOUNT_DATA) upload_dir = safe_child(data_root, slug, config.DIR_UPLOAD) output_base = safe_child(data_root, slug, config.DIR_OUTPUT) ``` 4. Explicitly reject absolute paths, path separators, `.` and `..`, even if additional validation is introduced elsewhere. 5. Separate unrelated workloads into distinct Modal volumes and grant the OCR function access only to the volume required for the current task. 6. Add tests covering `../models`, `../../`, absolute paths, repeated separators, encoded separators, and valid identifiers. ]]>