T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/wallet.py:17
- Finding
- Unsafe Wallet Persistence Permits Symlink Overwrites and Concurrent State Corruption<![CDATA[ ## Vulnerability Details **File Location**: `scripts/wallet.py:17-89`; related concurrent mutation occurs at `scripts/telegram_bot.py:90-93` **Vulnerability Type**: Predictable relative file path, non-atomic file replacement, and unsynchronized shared-state access **Risk Level**: Medium ### Vulnerable Code ```python # Local storage file WALLET_FILE = "molt_rpg_wallets.json" ``` ```python def load(self): """Load wallets from disk""" if os.path.exists(WALLET_FILE): try: with open(WALLET_FILE, 'r') as f: data = json.load(f) self.wallets = data.get('wallets', {}) # Load transactions tx_list = data.get('transactions', []) self.transactions = [ Transaction( id=t['id'], from_player=t['from_player'], to_player=t['to_player'], amount=t['amount'], transaction_type=t['transaction_type'], timestamp=t['timestamp'], note=t.get('note') ) for t in tx_list ] except Exception as e: print(f"Error loading wallets: {e}") self.wallets = {} self.transactions = [] def save(self): """Save wallets to disk""" data = { 'wallets': self.wallets, 'transactions': [ { 'id': t.id, 'from_player': t.from_player, 'to_player': t.to_player, 'amount': t.amount, 'transaction_type': t.transaction_type, 'timestamp': t.timestamp, 'note': t.note } for t in self.transactions ] } with open(WALLET_FILE, 'w') as f: json.dump(data, f, indent=2) ``` The Telegram bot also performs an unsynchronized read-modify-write s ...[truncated 3103 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Store the ledger in a fixed per-user application-data directory rather than the current working directory. 2. Create the storage directory with permissions limited to the Skill's OS account. 3. Reject symbolic links and non-regular destination files. Where supported, use `os.open` with `O_NOFOLLOW`. 4. Write serialized data to a securely created temporary file in the same directory, flush it, call `os.fsync`, and commit it with `os.replace`. 5. Apply an inter-process file lock around the complete read-modify-write transaction. 6. Avoid directly mutating `wallet.wallets` from Telegram handlers. Expose a synchronized wallet method that validates and commits rewards. 7. For concurrent Telegram or multi-agent operation, replace JSON persistence with SQLite and transactional updates. 8. Validate loaded JSON types and preserve a recoverable backup rather than silently resetting all state after a parsing error. 9. Create files with restrictive permissions such as mode `0600`, subject to platform support. ]]>