T09 · Insecure Skill Coding Practices
Error
- Location
scripts/account_manager.py:139- Finding
Account Name Path Traversal Enables Arbitrary Recursive Directory Deletion
- Content
View full analysis
bool: data = _load_accounts() if name in data["accounts"]: return False from datetime import datetime profile_dir = os.path.join(PROFILES_BASE, name) os.makedirs(profile_dir, exist_ok=True) data["accounts"][name] = { "alias": alias or name, "profile_dir": profile_dir, "created_at": datetime.now().isoformat(), } _save_accounts(data) return True ``` ```python def remove_account(name: str, delete_profile: bool = False) -> bool: data = _load_accounts() if name not in data["accounts"]: return False # Don't allow removing the default account if it's the only one if name == data.get("default_account") and len(data["accounts"]) == 1: return False profile_dir = data["accounts"][name].get("profile_dir", "") del data["accounts"][name] # If we removed the default, set a new default if name == data.get("default_account"): data["default_account"] = next(iter(data["accounts"].keys())) _save_accounts(data) # Optionally delete the profile directory if delete_profile and profile_dir and os.path.isdir(profile_dir): try: shutil.rmtree(profile_dir) except Exception: pass return True ``` ### Technical Analysis The account name is incorporated directly into a filesystem path using `os.path.join(PROFILES_BASE, name)`. The value is not constrained to a safe identifier and the resulting canonical path is not checked to ensure that it remains under `PROFILES_BASE`. A name containing parent-directory components, such as `../../target`, can escape the intended profile dire ...[truncated 1714 chars]- Remediation
View remediation
str: if not ACCOUNT_NAME_PATTERN.fullmatch(name): raise ValueError("Invalid account name") return name ``` 2. Canonicalize and verify every generated profile path: ```python base = os.path.realpath(PROFILES_BASE) candidate = os.path.realpath(os.path.join(base, validated_name)) if os.path.commonpath([base, candidate]) != base or candidate == base: raise ValueError("Profile path escapes the profile base directory") ``` 3. Repeat the canonical containment check immediately before recursive deletion. Do not rely solely on the path stored in `accounts.json`. 4. Reject symbolic links and prevent time-of-check/time-of-use substitution where feasible. Profile directories should be created and managed exclusively by this application. 5. Record an application-generated profile identifier instead of accepting filesystem path components from users. 6. Require explicit confirmation before profile deletion and report deletion failures rather than silently suppressing every exception. ]]>