T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/bet_tracker.py:36
- Finding
- Path Traversal Through Unvalidated Bet Date<![CDATA[ ## Vulnerability Details **File Location**: `scripts/bet_tracker.py`, lines 36-48; user-controlled value reaches these functions at lines 173 and 281 **Vulnerability Type**: Path traversal and unintended file access **Risk Level**: Medium ### Vulnerable Code ```python def _load_bet_file(date_str: str) -> Dict[str, Any]: path = BETS_DIR / f"{date_str}.json" if not path.exists(): return {"date": date_str, "slips": []} with open(path, "r", encoding="utf-8") as fh: return json.load(fh) def _save_bet_file(date_str: str, data: Dict[str, Any]): _ensure_dirs() path = BETS_DIR / f"{date_str}.json" with open(path, "w", encoding="utf-8") as fh: json.dump(data, fh, indent=2, ensure_ascii=False) ``` The unvalidated value reaches the vulnerable functions through the result-recording flow: ```python bet_data = _load_bet_file(date_str) ... _save_bet_file(date_str, bet_data) ``` It originates from an unrestricted command-line argument: ```python parser.add_argument("--date", type=str, help="Date (YYYY-MM-DD) for result mode") ... output = mark_result(args.date, args.slip_idx, args.pick_idx, args.result) ``` ### Technical Analysis Although `--date` is documented as using the `YYYY-MM-DD` format, the program does not enforce that format. The supplied string is interpolated directly into a path under `BETS_DIR`. A value containing traversal components, such as `../../some/target`, produces a path equivalent to: ```text data/bets/../../some/target.json ``` No canonicalization or containment check ensures that the resolved path remains inside `data/bets`. Consequently, the operating system resolves traversal components before the file is opened. In `mark_result`, a selected file is parsed as JSON, accessed through its expected `slips` structure, modified, and written back to the same attacker-selected path. Exploitation therefore requires the targeted file to exist, be readable and writable by the Skill process, ...[truncated 1561 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Strictly parse and normalize the date before using it in a filename: ```python from datetime import datetime def _validate_date(date_str: str) -> str: try: parsed = datetime.strptime(date_str, "%Y-%m-%d").date() except ValueError as exc: raise ValueError("Date must use the YYYY-MM-DD format") from exc normalized = parsed.isoformat() if normalized != date_str: raise ValueError("Date must be a canonical YYYY-MM-DD value") return normalized ``` 2. Construct file paths only from the normalized value and enforce directory containment: ```python def _bet_path(date_str: str) -> Path: normalized = _validate_date(date_str) root = BETS_DIR.resolve() path = (root / f"{normalized}.json").resolve() if path.parent != root: raise ValueError("Bet file path is outside the permitted directory") return path ``` 3. Use the centralized safe path function for both reads and writes: ```python def _load_bet_file(date_str: str) -> Dict[str, Any]: path = _bet_path(date_str) ... def _save_bet_file(date_str: str, data: Dict[str, Any]): _ensure_dirs() path = _bet_path(date_str) ... ``` 4. Validate the loaded JSON schema before mutation. Require `slips` to be a list and validate the expected fields and types for every selected record. 5. Use atomic writes through a temporary file created inside `data/bets`, followed by `os.replace`, to reduce corruption risk. 6. Create local data files with restrictive permissions where supported, such as owner-only read and write access. ]]>