T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/metrics_card.py:27
- Finding
- Unrestricted Output Path Allows Arbitrary File Overwrite<![CDATA[ ## Vulnerability Details **File Location**: `scripts/metrics_card.py`, lines 27–34 and 192 **Vulnerability Type**: Improper path validation and unrestricted file write **Risk Level**: Medium ### Vulnerable Code ```python ap.add_argument("--symbol", required=True, help="6-digit A-share code, e.g., 600406") ap.add_argument("--name", default="", help="Chinese name (optional)") ap.add_argument("--out", default="", help="Output markdown path") args = ap.parse_args() symbol = args.symbol.strip() name = args.name.strip() out_path = Path(args.out) if args.out else Path("notes/stocks/cards") / f"{symbol}.md" out_path.parent.mkdir(parents=True, exist_ok=True) ``` The resulting path is subsequently written without any containment or overwrite checks: ```python out_path.write_text("\n".join(lines) + "\n", encoding="utf-8") ``` ### Technical Analysis The documentation states that the stock symbol should be validated, but the implementation only removes surrounding whitespace. It does not enforce the documented six-digit A-share symbol format. When `--out` is supplied, the value is converted directly to a `Path`. Absolute paths, parent-directory components such as `../`, symbolic-link targets, and paths outside the intended `notes/stocks/cards` directory are all accepted. When `--out` is omitted, the unvalidated `symbol` is incorporated into the default output path. A symbol containing path separators or traversal components can therefore alter the destination. The call to `mkdir(parents=True, exist_ok=True)` may also create attacker-selected directory trees. Finally, `Path.write_text()` overwrites an existing destination by default. There is no canonical-path containment check, symbolic-link defense, confirmation requirement, or protection against replacing an existing file. ### Attack Path 1. An attacker or untrusted caller provides a crafted output argument, for example: ```bash python scripts/metrics_card.py --symbol 600406 --out /tmp/target- ...[truncated 1347 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Enforce the documented stock-symbol format before using it in a path: ```python import re if not re.fullmatch(r"[036]\d{5}", symbol): ap.error("--symbol must be a six-digit A-share code beginning with 0, 3, or 6") ``` 2. Restrict output files to an explicitly approved root directory: ```python output_root = Path("notes/stocks/cards").resolve() candidate = ( Path(args.out) if args.out else output_root / f"{symbol}.md" ) candidate = candidate.resolve() try: candidate.relative_to(output_root) except ValueError: ap.error("--out must remain inside notes/stocks/cards") ``` 3. Reject absolute paths and parent-directory components before resolution if callers are not intended to select arbitrary destinations. 4. Define a clear overwrite policy. Refuse to replace existing files by default, or require an explicit trusted `--force` option: ```python if candidate.exists() and not args.force: ap.error("Output file already exists; use --force to replace it") ``` 5. If the execution environment may contain attacker-controlled symbolic links, open the destination using platform-supported no-follow and exclusive-creation controls rather than relying solely on a pre-write existence check. 6. Run the Skill with minimal filesystem permissions so that only the designated card-output directory is writable. ]]>