T09 · Insecure Skill Coding Practices
Error
- Location
- src/index.ts:65
- Finding
- Ambiguous Amount Parsing Can Cause Transfers Far Larger Than Requested<![CDATA[ ## Vulnerability Details **File Location**: `src/index.ts:65-87`; related conflicting declarations in `skill.json:53-56`, `skill.json:76-79`, and `skill.json:118-121` **Vulnerability Type**: Ambiguous financial-unit handling **Risk Level**: High ### Vulnerable Code ```typescript async function parseHumanAmount(amount: string, token: string, kit?: ClankerKit): Promise<bigint> { let decimals = TOKEN_DECIMALS[token.toUpperCase()]; // For contract addresses not in TOKEN_DECIMALS, try on-chain lookup if (decimals === undefined && kit && token.startsWith('0x') && token.length === 42) { decimals = await kit.fetchTokenDecimals(token); } decimals = decimals ?? 18; // Contains a decimal point => definitely human-readable if (amount.includes('.')) { return parseUnits(amount, decimals); } // Pure integer: if it's very large (>10 digits), assume it's already in wei. // This handles backward compatibility for callers passing raw wei. if (/^\d+$/.test(amount) && amount.length > 10) { return BigInt(amount); } // Short integer like "1", "100" => treat as human-readable return parseUnits(amount, decimals); } ``` The runtime manifest describes affected parameters as raw units, for example: ```json "amount": { "type": "string", "description": "Amount in wei" } ``` The parser is used by asset-moving operations such as: ```typescript async send_tokens({ to, amount }: { to: string; amount: string }) { const kit = getClankerKit(); const amountWei = await parseHumanAmount(amount, 'MON', kit); const result = await kit.send(to as Address, amountWei); ``` ### Technical Analysis The public interface and implementation assign different meanings to integer amount strings. The manifest instructs an Agent to supply wei or the token's smallest unit, while `parseHumanAmount` treats any integer containing ten or fewer digits as a human-readable whole-token amount. For an asset with 18 decimals, the manifest-compatible input ` ...[truncated 2074 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Remove the string-length heuristic and define one unambiguous unit for every parameter. 2. Prefer separate schema fields or tools for distinct representations, such as `amount` for human-readable values and `amountWei` for raw values. 3. If human-readable amounts are retained, always apply `parseUnits` and update every `skill.json` and `SKILL.md` description accordingly. 4. If raw units are retained, always use `BigInt(amount)` after validating that the value is a non-negative decimal integer. 5. Reject negative values, scientific notation, malformed decimal strings, excessive precision, and values exceeding configured transaction limits. 6. Require explicit confirmation that displays both the human-readable amount and raw-unit value before asset-moving operations. 7. Add tests covering `"0"`, `"1"`, `"100"`, `"10000000000"`, `"100000000000"`, decimal inputs, and tokens with non-18 decimal precision. 8. Make restrictive policies mandatory before exposing transfer, swap, staking, or arbitrary-transaction tools. ]]>