T09 · Insecure Skill Coding Practices
Error
- Location
scripts/arxiv_client.py:113- Finding
Path Traversal Through Unvalidated Paper Identifiers
- Content
View full analysis
Optional[dict]: """Get cached paper metadata.""" cache_file = METADATA_DIR / f"{paper_id}.json" if cache_file.exists(): with open(cache_file, 'r') as f: return json.load(f) return None ``` ```python def fetch_paper_pdf(paper_id: str) -> Tuple[bool, str]: """Download paper PDF.""" ensure_cache_dirs() # Clean paper ID clean_id = paper_id.replace("arXiv:", "").strip() # Check cache pdf_path = PAPERS_DIR / f"{clean_id}.pdf" if pdf_path.exists(): return True, str(pdf_path) ``` ### Technical Analysis The `paper_id` argument originates from the command line and is inserted directly into paths under `METADATA_DIR` and `PAPERS_DIR`. Removing the optional `arXiv:` prefix and surrounding whitespace does not prevent path separators, absolute paths, or `..` traversal components. The query validator is not applied to paper IDs used by the `fetch` and `log` commands. Consequently, a value such as `../../target` causes Python's `pathlib` path composition to resolve outside the intended cache directory. The metadata path always receives a `.json` suffix and the PDF path always receives a `.pdf` suffix, which limits—but does not eliminate—the files that can be targeted. No canonical path containment check is performed before reading or writing. ### Attack Path 1. An attacker gains the ability to invoke the skill or influence the paper ID supplied to its CLI. 2. The attacker supplies an identifier containing traversal components, such as `../../some/path/target`. 3. `get_cached_metadata()` constructs a path outside `METADATA_DIR`, ending in `.json`. 4. If that file exists and contains JSON compatible with `ArxivPaper`, the ...[truncated 809 chars]- Remediation
View remediation
str: clean_id = value.removeprefix("arXiv:").strip() if not ARXIV_ID_RE.fullmatch(clean_id): raise ValueError("Invalid ArXiv paper ID") return clean_id ``` 3. Resolve every generated path and verify that it remains beneath the expected directory: ```python def contained_path(base: Path, filename: str) -> Path: base = base.resolve() candidate = (base / filename).resolve() if candidate.parent != base: raise ValueError("Path escapes the configured storage directory") return candidate ``` 4. Apply the same validation consistently to cache reads, metadata writes, cache checks, and PDF downloads. 5. Open newly downloaded files safely and avoid silently overwriting existing files outside the intended lifecycle. 6. Add tests for `../`, absolute paths, backslashes, encoded traversal strings, legacy IDs containing slashes, and identifiers with version suffixes. ]]>