T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/codex-accounts.py:556
- Finding
- Unrestricted account names allow path traversal and unintended credential file access<![CDATA[ ## Vulnerability Details **File Location**: `scripts/codex-accounts.py:556-568, 1005-1019, 1266-1280, 1578-1599, 1641-1644, 1661-1685` **Vulnerability Type**: Path traversal and unsafe credential-file handling **Risk Level**: High ### Vulnerable Code Account paths are constructed directly from unvalidated names: ```python def _resolve_unique_name_path(base_name: str) -> tuple[str, Path]: base = (base_name or "account").strip() or "account" target = ACCOUNTS_DIR / f"{base}.json" if not target.exists(): return base, target suffix = 2 while True: candidate_name = f"{base}-{suffix}" candidate = ACCOUNTS_DIR / f"{candidate_name}.json" if not candidate.exists(): return candidate_name, candidate suffix += 1 ``` The `compare` command uses caller-controlled names as read paths: ```python def cmd_compare(name_a: str, name_b: str, json_mode: bool = False): path_a = ACCOUNTS_DIR / f"{name_a}.json" path_b = ACCOUNTS_DIR / f"{name_b}.json" if not path_a.exists(): print(f"❌ Account snapshot not found for '{name_a}': {path_a}") return if not path_b.exists(): print(f"❌ Account snapshot not found for '{name_b}': {path_b}") return with open(path_a, "r") as f: a = json.load(f) with open(path_b, "r") as f: b = json.load(f) ``` The `use` command similarly accepts an unvalidated source path and copies it over the active authentication file: ```python def cmd_use(name, sync_openclaw: bool = False, agent_names: list[str] | None = None): ensure_dirs() source = ACCOUNTS_DIR / f"{name}.json" if not source.exists(): print(f"❌ Account '{name}' not found.") print("Available accounts:") for f in _iter_account_snapshot_files(): print(f" - {f.stem}") return # Backup current if it's not saved? # Maybe risky to overwrite silently, but that's what a switcher does ...[truncated 6818 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Apply strict account-name validation** Use a centralized validator and permit only a conservative filename format, for example: ```python import re ACCOUNT_NAME_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$") def validate_account_name(name: str) -> str: value = name.strip() if not ACCOUNT_NAME_RE.fullmatch(value): raise ValueError("Invalid account name") return value ``` Apply this validator to `add --name`, `save`, `use`, `compare`, and all JWT-derived names. 2. **Enforce path containment** Construct paths through one helper, resolve the account directory and candidate path, and verify that the candidate is a direct child: ```python def account_path(name: str) -> Path: safe_name = validate_account_name(name) base = ACCOUNTS_DIR.resolve() candidate = (base / f"{safe_name}.json").resolve(strict=False) if candidate.parent != base: raise ValueError("Account path escapes the accounts directory") return candidate ``` Replace every direct expression of the form `ACCOUNTS_DIR / f"{name}.json"` with this helper. 3. **Reject symbolic links** Before reading or writing, use `lstat()` or `Path.is_symlink()` to reject symlink sources and destinations. Where supported, open files with no-follow semantics. 4. **Use atomic, restrictive credential writes** Write credentials to a temporary file created inside `ACCOUNTS_DIR`, set mode `0600`, flush and synchronize it, and atomically replace the validated destination. Explicitly create `ACCOUNTS_DIR` with mode `0700`. 5. **Treat JWT claims as untrusted input** Sanitize email local parts and user IDs before using them as filenames. If sanitization produces an empty or invalid name, require an explicit safe name instead of preserving path characters. 6. **Limit implicit writes** Reconsider running `sync_current_login_to_snapshot()` on every command ...[truncated 439 chars]