T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/transfer.mjs:21
- Finding
- Insufficient Validation of Financial Transfer Amount## Vulnerability Details **File Location**: `scripts/transfer.mjs`, lines 21–25 and 45–50 **Vulnerability Type**: Improper input validation and unsafe numeric parsing **Risk Level**: Medium ### Vulnerable Code ```js if (!args.to || !args.amount) { console.error('Usage: node transfer.mjs --to <recipient> --amount <number> [--memo "reason"]'); process.exit(1); } ``` ```js body: JSON.stringify({ to: args.to, amount: parseFloat(args.amount), memo: args.memo, }), ``` ### Technical Analysis The transfer script verifies only that `--amount` is present. It then uses `parseFloat()` without checking whether the entire input is a valid decimal number or whether the parsed value is finite, positive, within an acceptable range, and compatible with the token's supported precision. JavaScript's `parseFloat()` accepts numeric prefixes. For example, `10abc` is silently converted to `10`. Inputs such as `0`, negative numbers, extremely large values, and values that produce non-finite results are not rejected locally. Although `JSON.stringify()` converts non-finite numeric values to `null`, the resulting malformed request is still submitted to the financial API. Correct server-side validation may prevent an invalid transaction, but the client should not rely exclusively on undocumented remote safeguards for a value-changing operation. ### Attack Path 1. A user, automation workflow, or calling Agent invokes the script with a crafted or accidentally malformed value, such as `--amount 10abc`, `--amount -10`, or an excessively large number. 2. The presence check succeeds because the argument is a non-empty string. 3. `parseFloat()` coerces the input without requiring full-string validity. 4. The resulting value is placed in the JSON request body. 5. The script sends the request to the authenticated transfer endpoint. 6. If the service accepts the coerced or insufficiently constrained value, an unint ...[truncated 855 chars]
- Remediation
- ## Remediation Suggestions - Validate the complete amount string against the token's canonical decimal syntax rather than accepting a numeric prefix. - Convert the value once and reject it unless `Number.isFinite(amount)` is true. - Require the amount to be strictly greater than zero. - Enforce documented minimum and maximum transfer limits. - Enforce the token's supported decimal precision, preferably by converting decimal input to an integer quantity in the smallest token unit rather than relying on binary floating-point arithmetic. - Reject exponential notation unless the API explicitly supports it. - Consider requiring explicit user confirmation that displays the normalized recipient, amount, agent ID, and memo before submitting the transfer. - Preserve server-side validation as a separate mandatory control. Example hardening pattern: ```js const amountText = args.amount; if (!/^(?:0|[1-9]\d*)(?:\.\d{1,SUPPORTED_PRECISION})?$/.test(amountText)) { console.error('Amount must be a valid positive decimal with supported precision.'); process.exit(1); } const amount = Number(amountText); if (!Number.isFinite(amount) || amount <= 0 || amount > MAX_TRANSFER) { console.error('Amount is outside the permitted transfer range.'); process.exit(1); } ``` Replace `SUPPORTED_PRECISION` and `MAX_TRANSFER` with limits defined by the Quack Network API.