T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/tracking_commit.py:1068
- Finding
- Symlink-Following Cleanup Can Delete Files Outside the Project<![CDATA[ ## Vulnerability Details **File Location**: `scripts/tracking_commit.py`, lines 1068-1073 **Vulnerability Type**: Unrestricted file deletion through a symlinked managed directory **Risk Level**: High ### Vulnerable Code ```python expected_character_files = { Path(relative).name for relative in views if relative.startswith("角色状态/") } character_dir = tracking / "角色状态" character_dir.mkdir(parents=True, exist_ok=True) for path in character_dir.glob("*.md"): if path.name not in expected_character_files: path.unlink() ``` ### Technical Analysis The `write_views()` cleanup routine assumes that `追踪/角色状态` is a real directory located beneath the selected project. It neither checks the directory with `lstat()` nor verifies that its resolved path remains inside the resolved project root. If a crafted project contains `追踪/角色状态` as a symbolic link to another directory, `mkdir(..., exist_ok=True)` accepts the existing linked directory. The subsequent `glob("*.md")` enumeration follows that link, and `path.unlink()` removes Markdown files from the linked external directory whenever their names are absent from `expected_character_files`. The deletion operation runs with the privileges of the user invoking the Skill. Exploitation does not require command injection or shell execution; it relies entirely on filesystem path resolution. ### Attack Path 1. An attacker prepares or modifies a story project. 2. The attacker replaces `追踪/角色状态` with a symbolic link to a directory containing files the victim can modify. 3. The victim opens the project and invokes a documented tracking initialization or commit workflow. 4. `write_views()` follows the symbolic link and enumerates `*.md` files in the external target directory. 5. Every enumerated file not matching an expected generated character-state filename is deleted. 6. The files are removed using the victim process's filesystem permissions. ### Impact Assessment An attacker can cause deletion of Markdo ...[truncated 455 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Reject symbolic links for every managed tracking directory: ```python character_dir = tracking / "角色状态" if character_dir.is_symlink(): raise TrackingError("character-state directory must not be a symbolic link") ``` 2. Resolve and enforce directory containment before reading, writing, or deleting: ```python tracking_real = tracking.resolve(strict=True) character_real = character_dir.resolve(strict=True) try: character_real.relative_to(tracking_real) except ValueError as exc: raise TrackingError("character-state directory escapes tracking root") from exc ``` 3. Use `os.lstat()` or `Path.lstat()` so validation examines the directory entry itself rather than following a link. 4. Securely create managed directories during initialization and record their expected identity. Refuse to proceed if an existing path is not a real directory owned or controlled by the invoking user. 5. Before deleting each stale file, verify that: - The file is a regular file and not a symbolic link. - Its resolved parent is the validated character-state directory. - Its name matches the exact generated-file naming convention. 6. Prefer an allowlisted manifest of files previously generated by the tool rather than deleting every unexpected `*.md` file. This avoids deleting unrelated user-created files even inside the legitimate managed directory. 7. Add regression tests using: - A symlinked `角色状态` directory. - Symlinked individual Markdown files. - Nested paths resolving outside the project. - Unrelated user-created Markdown files in the managed directory. ]]>