T09 · Insecure Skill Coding Practices
Error
- Location
scripts/deepresearch.py:25- Finding
Path Traversal Through Unvalidated Ledger Slug
- Content
View full analysis
Path: if slug: return ROOT / slug current = ROOT / 'current' if current.exists(): target = current.read_text().strip() if target: return ROOT / target raise SystemExit('No slug supplied and .deepresearch/current is missing. Run init first.') ``` ```python def write_json(path: Path, data: Any) -> None: path.parent.mkdir(parents=True, exist_ok=True) tmp = path.with_suffix(path.suffix + '.tmp') tmp.write_text(json.dumps(data, indent=2, ensure_ascii=False) + '\n') tmp.replace(path) ``` ```python def cmd_init(args: argparse.Namespace) -> None: slug = args.slug or slugify(args.question) path = ROOT / slug data = ledger(path) data['question'] = args.question data['slug'] = slug if not data['lanes']: data['lanes'] = [ {'id': 'L001', 'name': 'background', 'question': 'What is the necessary context?', 'status': 'open'}, {'id': 'L002', 'name': 'evidence', 'question': 'What is the strongest evidence?', 'status': 'open'}, {'id': 'L003', 'name': 'counterevidence', 'question': 'What contradicts or weakens the claim?', 'status': 'open'}, {'id': 'L004', 'name': 'synthesis', 'question': 'What conclusion follows from the evidence?', 'status': 'open'}, ] save(path, data) ``` ### Technical Analysis The `--slug` command-line argument is treated as a filesystem path component without validation or canonical containment checks. In Python's `pathlib`, joining `ROOT` with an absolute path discards `ROOT`; relative components such as `..` can also escape the intended `.deepresearch` di ...[truncated 2760 chars]- Remediation
View remediation
str: if not SLUG_RE.fullmatch(slug): raise SystemExit('Invalid slug.') return slug ``` This rejects absolute paths, directory separators, `..`, empty values, and platform-specific path syntax. 2. **Enforce canonical containment before every read or write** Resolve the ledger root and candidate destination, then verify that the destination remains beneath the root: ```python ROOT = Path('.deepresearch').resolve() def safe_run_dir(slug: str) -> Path: slug = validate_slug(slug) candidate = (ROOT / slug).resolve() if candidate.parent != ROOT: raise SystemExit('Ledger path escapes .deepresearch.') return candidate ``` A robust implementation may use `candidate.relative_to(ROOT)` and reject `ValueError`. Do not rely exclusively on textual checks for `..`. 3. **Validate persisted state** Apply the same slug validation and canonical containment checks to the value read from `.deepresearch/current`. Treat this file as untrusted or potentially corrupted input. 4. **Address symlink traversal** Before writing, reject symlinks in the path chain or use secure directory-relative file operations that prevent following attacker-controlled links. Recheck containment as close as possible to the file operation to reduce time-of-check/time-of-use exposure. 5. **Avoid accepting path semantics where only identifiers are required** Keep `--slug` as a logical identifier rather than a path. If configurable storage paths are needed in the future, expose them through a separate, explicitly authorized option with dedicated validation. 6. **Add regression tests** Verify rejection of at least: - `../outside` - `../../ ...[truncated 341 chars]