T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/test_runner.py:288
- Finding
- Fixed Output Path Permits Symlink-Based File Overwrite## Vulnerability Details **File Location**: `scripts/test_runner.py`, lines 288-291 **Vulnerability Type**: CWE-59 — Improper Link Resolution Before File Access **Risk Level**: Medium ### Vulnerable Code ```python # Save to JSON result_dict = asdict(result) with open("/home/gem/.aily/workspace/skills/voight-kampff-test/results/demo_result.json", "w") as f: json.dump(result_dict, f, indent=2, ensure_ascii=False) ``` ### Technical Analysis The demonstration routine writes its result to a fixed, absolute path using `open(..., "w")`. This mode follows symbolic links and truncates the resolved destination before writing. The code does not verify that the destination is a regular file, validate ownership, prevent symbolic-link traversal, or create the output securely. If another local user can control the `results` directory or replace `demo_result.json` with a symbolic link, running the script under a more privileged account can overwrite a file accessible to that account. The written content is constrained to the generated JSON report, so this is not an arbitrary-content write; nevertheless, truncation and replacement of the target's contents may cause data loss or service disruption. The hardcoded environment-specific path can also cause execution failure when the directory does not exist, although that reliability problem is secondary to the unsafe file-handling issue. ### Attack Path 1. An attacker obtains write access to `/home/gem/.aily/workspace/skills/voight-kampff-test/results/` or otherwise controls `demo_result.json`. 2. The attacker creates a symbolic link from `demo_result.json` to a file writable by the account expected to run the skill: ```bash ln -s /path/to/target /home/gem/.aily/workspace/skills/voight-kampff-test/results/demo_result.json ``` 3. A more privileged user or agent runs: ```bash python scripts/test_runner.py ``` 4. Python follows the symbolic link ...[truncated 922 chars]
- Remediation
- ## Remediation Suggestions - Do not write demonstration output to a hardcoded absolute path. Accept an explicit output path from the caller or use a directory owned exclusively by the current user. - Create the destination directory with restrictive permissions and verify its ownership before writing. - Refuse to follow symbolic links. On supported platforms, open the file using `os.open()` with `O_NOFOLLOW`, `O_CREAT`, and `O_EXCL`, then wrap the descriptor with `os.fdopen()`. - Validate the opened file with `os.fstat()` and ensure that it is a regular file owned by the expected user. - Prefer atomic output: securely create a temporary file in the same trusted directory, flush and synchronize it, and then use `os.replace()` after validating the destination. - Apply restrictive file permissions, such as `0o600`, if reports may contain subject responses or other sensitive information. - For a demonstration script that does not need persistence, print the report only or use Python's `tempfile` module rather than a predictable shared path. A hardened approach should resemble: ```python import os from pathlib import Path output_dir = Path.home() / ".local" / "share" / "voight-kampff-test" output_dir.mkdir(mode=0o700, parents=True, exist_ok=True) output_path = output_dir / "demo_result.json" flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL if hasattr(os, "O_NOFOLLOW"): flags |= os.O_NOFOLLOW fd = os.open(output_path, flags, 0o600) with os.fdopen(fd, "w", encoding="utf-8") as f: json.dump(result_dict, f, indent=2, ensure_ascii=False) ``` If overwriting an existing report is required, use a securely created temporary file and an atomic replacement strategy rather than opening a predictable destination directly.