T09 · Insecure Skill Coding Practices
Error
- Location
- src/axiotic_experiments/github_store.py:40
- Finding
- Unvalidated results prefix permits filesystem path traversal, deletion, and overwrite<![CDATA[ ## Vulnerability Details **File Location**: `src/axiotic_experiments/spec.py:340`; `src/axiotic_experiments/github_store.py:40-55` **Vulnerability Type**: Path traversal and unsafe recursive deletion **Risk Level**: High ### Vulnerable Code ```python # src/axiotic_experiments/spec.py github=GitHubSpec( repo=str(_required(github, "repo", "github")), results_branch=str(_required(github, "results_branch", "github")), results_prefix=str(github.get("results_prefix", "experiments/results")), ), ``` ```python # src/axiotic_experiments/github_store.py class GitHubResultStore: def __init__(self, *, repo_dir: Path, results_prefix: str): self.repo_dir = repo_dir.expanduser().absolute() self.results_prefix = results_prefix.strip("/") def stage(self, bundle_dir: Path, *, run_id: str) -> Path: assert_no_secret_material(bundle_dir) destination = self.repo_dir / self.results_prefix / run_id destination.mkdir(parents=True, exist_ok=True) for source in bundle_dir.iterdir(): target = destination / source.name if source.is_dir(): if target.exists(): shutil.rmtree(target) shutil.copytree(source, target) elif source.is_file(): shutil.copy2(source, target) return destination ``` ### Technical Analysis The specification parser accepts `[github].results_prefix` without rejecting absolute paths or `..` path components. Calling `strip("/")` removes leading and trailing separators but does not neutralize traversal sequences. `GitHubResultStore.stage()` joins this attacker-controlled value to `repo_dir` and then performs directory creation, recursive deletion, and file copying. A value such as `../../victim` can cause the normalized destination to escape the intended repository. The recursive `shutil.rmtree(target)` operation makes this more severe than a simple arbitrary write: an existing direc ...[truncated 968 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Reject absolute `results_prefix` values and any path containing `..` components during specification validation. - Resolve the final destination before performing filesystem operations and verify that it remains under `repo_dir`: ```python base = self.repo_dir.resolve() destination = (base / self.results_prefix / run_id).resolve() if not destination.is_relative_to(base): raise GitHubStoreError("results destination escapes repository root") ``` - Apply the same containment check to every target before deletion or copying. - Validate `results_prefix` against a conservative repository-relative path format. - Add regression tests covering `../`, nested traversal, absolute paths, repeated separators, and symlink-assisted escapes. - Avoid recursively deleting an existing target unless it has first been proven to reside under the expected results directory. ]]>