T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/init_twin_profile.py:86
- Finding
- Symbolic-Link Following Allows Out-of-Directory File Overwrite## Vulnerability Details **File Location**: `scripts/init_twin_profile.py:86-90, 100-128` **Vulnerability Type**: Symbolic-link following and time-of-check/time-of-use file-write weakness **Risk Level**: Medium ### Vulnerable Code ```python def validate_targets(output_dir: Path, force: bool) -> None: conflicts = [] for name in TEMPLATE_NAMES: target = output_dir / name if target.exists() and not force: conflicts.append(str(target)) if conflicts: joined = "\n".join(f"- {item}" for item in conflicts) raise FileExistsError( "Refusing to overwrite existing files without --force:\n" f"{joined}" ) def main() -> int: args = parse_args() out_dir = Path(args.output_dir).expanduser().resolve() source_dir = template_dir() # ... out_dir.mkdir(parents=True, exist_ok=True) try: validate_targets(out_dir, args.force) except FileExistsError as exc: print(f"[ERROR] {exc}", file=sys.stderr) return 1 for name in TEMPLATE_NAMES: source = template_path(source_dir, name, language) if not source.is_file(): print(f"[ERROR] Missing template: {source}", file=sys.stderr) return 1 target = out_dir / name target.write_text(render_template(source, args.user_name), encoding="utf-8") print(f"[OK] Wrote {target}") ``` ### Technical Analysis The initializer checks target paths with `Path.exists()` and later writes them using `Path.write_text()`. It does not reject symbolic links or open destination files with no-follow semantics. `Path.write_text()` follows a destination symbolic link. When `--force` is enabled, validation permits any existing destination, including a symbolic link. A dangling symbolic link can also bypass the non-force check because `Path.exists()` normally returns false when the link's destination does not exist. The validation and write are separate filesystem ...[truncated 1488 chars]
- Remediation
- ## Remediation Suggestions 1. Reject every destination that is a symbolic link, including dangling links. Use `lstat()` or equivalent link-aware checks rather than relying only on `exists()`. 2. Open destination files atomically with no-follow semantics. On supported platforms, use `os.open()` with `O_NOFOLLOW` and appropriate creation flags, then write through the returned file descriptor. 3. When creating new files, use exclusive creation such as `O_CREAT | O_EXCL` to prevent an existing entry from being followed or replaced silently. 4. For approved overwrites, securely open the existing destination without following links and verify through `fstat()` that it is a regular file. 5. Keep validation and opening atomic. Do not depend on a path check followed by a separate path-based write, because an attacker can alter the directory entry between those operations. 6. Consider restricting output directories to an approved repository subtree unless arbitrary output locations are required. If arbitrary destinations remain supported, clearly document that the directory must not be writable by untrusted users. 7. Add automated tests covering existing symlinks, dangling symlinks, non-regular files, `--force` behavior, and destination replacement during initialization.
