T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/merge_results.py:56
- Finding
- Path Traversal Through an Unvalidated Target Slug<![CDATA[ ## Vulnerability Details **File Location**: `scripts/merge_results.py`, lines 56–68 **Vulnerability Type**: Path traversal and unintended local file disclosure **Risk Level**: Medium ### Vulnerable Code ```python for t in targets: slug = t.get("slug", "") if not slug: defects.append({"slug": "?", "issues": ["target_without_slug"]}) continue f = result_dir / f"{slug}.json" if not f.exists(): missing.append(slug) continue try: data = json.loads(f.read_text(encoding="utf-8")) except Exception as e: defects.append({"slug": slug, "issues": [f"json_parse_error: {e}"]}) continue ``` ### Technical Analysis The `slug` value is loaded from the run's `targets.json` file and inserted directly into a filesystem path: ```python f = result_dir / f"{slug}.json" ``` The code does not enforce a filename-safe slug format and does not resolve the resulting path to verify that it remains inside `result_dir`. Consequently, a slug containing parent-directory components such as `../` can escape the intended result directory. Depending on `pathlib` path-composition behavior, an absolute path supplied as a slug can also replace the intended base path. The `.json` suffix limits the issue to paths ending in that suffix, and the selected file must contain valid JSON to be processed successfully. Nevertheless, many application configuration, metadata, credential, and state files use JSON and could be exposed. ### Attack Path 1. An attacker gains the ability to supply or modify a run's `targets.json`, such as through an untrusted tabular intake or target-list input. 2. The attacker assigns a target a traversal slug, for example: ```json { "slug": "../../sensitive/config", "name": "Injected target" } ``` 3. A user or agent executes the documented merge operation: ```bash python scripts/merge_results.py --run-dir /path/to/run ``` 4. The script constructs a path ...[truncated 944 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Enforce a conservative slug allowlist before any path construction: ```python SLUG_RE = re.compile(r"^[A-Za-z0-9_-]+$") if not isinstance(slug, str) or not SLUG_RE.fullmatch(slug): defects.append({"slug": str(slug), "issues": ["invalid_slug"]}) continue ``` 2. Resolve both the result directory and candidate file, then enforce direct containment: ```python result_root = result_dir.resolve() candidate = (result_root / f"{slug}.json").resolve() if candidate.parent != result_root: defects.append({"slug": slug, "issues": ["slug_path_escape"]}) continue ``` 3. Reject absolute paths, path separators, `.` and `..` components explicitly, even if an allowlist is used. 4. Apply the same slug validation when creating `targets.json` and when workers select their output paths. Validation at ingestion improves usability, while validation at file access remains necessary as a security boundary. 5. Add regression tests covering: - `../outside` - nested paths such as `subdir/file` - absolute paths - Windows separators and drive paths - valid slugs containing letters, numbers, underscores, and hyphens ]]>