T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/crypto_utils.py:63
- Finding
- Wallet Name Path Traversal and Symbolic-Link File Overwrite<![CDATA[ ## Vulnerability Details **File Location**: `scripts/crypto_utils.py:63-84` **Vulnerability Type**: Path traversal and unsafe symbolic-link following **Risk Level**: High ### Vulnerable Code ```python def save_wallet(name: str, address: str, encrypted_key: dict, chain_type: str): """Save an encrypted wallet to disk.""" wallet_file = KEYSTORE_DIR / f"{name}.json" data = { "name": name, "address": address, "chain_type": chain_type, "encrypted_key": encrypted_key } with open(wallet_file, 'w') as f: json.dump(data, f, indent=2) wallet_file.chmod(0o600) # Owner read/write only return str(wallet_file) def load_wallet(name: str) -> dict: """Load an encrypted wallet from disk.""" wallet_file = KEYSTORE_DIR / f"{name}.json" if not wallet_file.exists(): raise FileNotFoundError(f"Wallet '{name}' not found") with open(wallet_file, 'r') as f: return json.load(f) ``` Wallet names reach these functions from command-line arguments at `scripts/wallet_manager.py:82` and `scripts/wallet_manager.py:88`. ### Technical Analysis The application directly interpolates an untrusted wallet name into a filesystem path without validating its characters, resolving the resulting path, or confirming that it remains under `KEYSTORE_DIR`. A wallet name containing parent-directory components, such as `../../target`, causes the resulting path to escape the intended keystore directory. Because the `.json` suffix is appended automatically, any user-writable JSON path that can be represented relative to the keystore can be targeted. The write operation uses the ordinary `open(..., 'w')` interface. It therefore follows an existing symbolic link and truncates its target. The subsequent `chmod(0o600)` may also alter the permissions of the resolved target. The corresponding load operation has the same traversal weakness and can open JSON files outside the keystore. ### Attack Pa ...[truncated 1546 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Enforce a strict wallet-name allowlist, for example `^[A-Za-z0-9_-]{1,64}$`. 2. Reject names containing path separators, `.` or `..` path components, control characters, and absolute paths. 3. Resolve the candidate path and verify that its parent is exactly the resolved keystore directory. 4. Create files atomically with `os.open()` and flags such as `O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW`, where supported. 5. Apply mode `0o600` at creation time rather than correcting permissions after opening the file. 6. Explicitly create and verify the keystore directory with mode `0o700`. 7. Refuse to read non-regular files and symbolic links. 8. Add tests covering absolute paths, nested traversal, symbolic links, repeated names, and race conditions. ]]>