T09 · Insecure Skill Coding Practices
Error
- Location
src/utils/storage.ts:147- Finding
Unverified Local Strategy Address Can Redirect Token Approvals and Deposits
- Content
View full analysis
Vulnerability Details
File Location:
src/utils/storage.ts:68-74, 147-149src/clients/blockchain.ts:151-155, 166-188src/commands/deposit.ts:94-101, 249-265, 284-293
Vulnerability Type: Improper validation of a security-sensitive contract address loaded from local storage
Risk Level: HighVulnerable Code
The strategy configuration file is written without explicit restrictive permissions:
ts export function saveLocalStrategies(data: LocalStrategiesData): void { writeFileSync(getStrategiesPath(), JSON.stringify(data, null, 2)); }Strategy addresses are then read directly from the local JSON data:
ts export function getLocalStrategyAddresses(walletAddress: Address): Address[] { const strategies = getLocalStrategiesForWallet(walletAddress); return Object.values(strategies).map((entry) => entry.address); }Locally supplied addresses are merged into the set of candidate strategies without calling the available registry validation function:
ts // 2. Merge local strategies const localAddresses = getLocalStrategyAddresses(walletAddress); for (const addr of localAddresses) { if (!results.some((a) => a.toLowerCase() === addr.toLowerCase())) { results.push(addr); } } return results;Candidate contracts are accepted solely because their
token()method returns the requested token address:ts export async function findStrategyForToken( publicClient: PublicClient<Transport, Chain>, walletAddress: Address, tokenAddress: Address | null ): Promise<Address | null> { if (!tokenAddress) return null; const strategies = await getAllStrategies(publicClient, walletAddress); for (const addr of strategies) { try { const stratToken = await publicClient.readContract({ address: addr, abi: STRATEGY_ABI, functionName: 'token', }); if (stratToken.toLow ...[truncated 4330 chars]- Remediation
View remediation
Remediation Suggestions
- Require on-chain registry validation before every asset-moving operation. Call
isUserStrategy(walletAddress, strategyAddress)and reject the address unless it returnstrue. - Validate ownership. Require
owner()to equal the active wallet before granting any allowance or invokingdeposit. - Validate contract provenance. Confirm that the contract was deployed by an approved factory and that its runtime bytecode, proxy implementation, strategy type, and underlying token match trusted values.
- Do not treat local storage as an authority. Use the JSON file only as a cache or discovery hint. A local entry must never become an approval recipient until independently validated on-chain.
- Handle legitimate local-only deployments securely. If newly created strategies are temporarily absent from the registry, validate the factory transaction receipt and emitted deployment event, bind the resulting address to the expected owner and factory, and revalidate those properties before each deposit.
- Harden local file storage. Create
~/.config/mamowith mode0700, create configuration files with mode0600, use atomic replacement, reject symbolic links, and validate the parsed schema and Ethereum addresses. - Fail closed. If registry, ownership, bytecode, or factory validation is unavailable, do not approve tokens or execute a deposit.
- Add regression tests. Include tests proving that a local contract with a matching
token()response is rejected when registry membership, ownership, or trusted provenance cannot be established. - Correct the documentation. Either implement the stated registry verification guarantee or clearly disclose the fallback trust model and its security limitations.
- Require on-chain registry validation before every asset-moving operation. Call