T09 · Insecure Skill Coding Practices
Warning
- Location
- src/skill.py:101
- Finding
- Unvalidated arXiv ID Can Cause Output Path Traversal## Vulnerability Details **File Location**: `src/skill.py`, lines 101-114 **Vulnerability Type**: Path traversal through unsafe filename construction **Risk Level**: Medium ```python pdf_url = f"https://arxiv.org/pdf/{arxiv_id}.pdf" safe_title = "".join(c if c.isalnum() or c in " -_" else "_" for c in (title[:40] or arxiv_id)) filename = f"{arxiv_id}_{safe_title}.pdf" filepath = self.output_dir / filename if filepath.exists(): return {"status": "skipped", "path": str(filepath), "size": 0} try: resp = self.session.get(pdf_url, timeout=120) if resp.status_code == 200: self.output_dir.mkdir(parents=True, exist_ok=True) with open(filepath, "wb") as f: f.write(resp.content) ``` ### Technical Analysis The externally supplied `arxiv_id` is inserted directly into `filename`. Only the title-derived portion is sanitized. Consequently, an ID containing path separators, parent-directory components, or platform-specific path syntax can cause `filepath` to point outside `self.output_dir`. The public `download_by_arxiv_ids()` entry point accepts arbitrary strings and forwards them to this method without validating them against supported arXiv identifier formats. `Path` does not confine joined paths to the original base directory. For example, parent-directory components can be resolved by the operating system when the file is opened. Exploitation is conditional on the constructed arxiv.org request returning HTTP status 200 and on any required intermediate directories existing. These conditions reduce reliability but do not establish a safe filesystem boundary. The implementation also does not resolve the destination and verify that it remains beneath the configured output directory. ### Attack Path 1. An attacker supplies a crafted string containing path traversal components through `download_by_arxiv_ids()`. 2. The value is passed unchanged to `ArxivDownloader.download_pdf()`. 3 ...[truncated 963 chars]
- Remediation
- ## Remediation Suggestions 1. Validate each identifier before constructing either the URL or filename. Permit only documented modern and legacy arXiv identifier formats using a full-string match. 2. Explicitly reject `/`, `\`, null bytes, parent-directory components, URL query markers, fragments, and unexpected percent-encoded characters. 3. Construct the filename exclusively from the validated identifier rather than retaining any raw user-supplied path characters. 4. Resolve both the base directory and candidate destination, then enforce containment before opening the file: ```python base = self.output_dir.resolve() destination = (base / filename).resolve() if destination.parent != base: raise ValueError("Invalid output path") ``` 5. Use `urllib.parse.quote()` with an empty safe-character set, or an equivalent URL-path encoder, after identifier validation when constructing the remote URL. 6. Add security tests covering `../`, absolute paths, Windows separators, URL fragments, query strings, percent encoding, and malformed legacy identifiers. 7. Consider opening new files in exclusive mode (`"xb"`) or using an atomic temporary-file replacement strategy to reduce unintended overwrites and partial files.