Description

The codebase generally lacks input sanity checks, which, even assuming no malicious behavior, could prevent issues in case of unintentional mistakes.

Instances of missing checks include (but are not limited to) the following:

• When inserting a perpetual order, the tpPrice is not required to be greater (or smaller, depending on whether the order is to increase or decrease the position) than slPrice.

• Functions to alter configuration do not enforce sensible numbers for fee parameters, which could even be greater than 100%.

• Lastly, FeedPrices does not check that the prices map contains only IDs of listed assets.

Impact

The lack of sanity checks allows the pool users (both privileged and unprivileged) to lose funds or cause damage, even via accidental actions.

Recommendations

Consider adding sanity checks to message parameters where possible. Even assuming honest behavior from the sender of a message, some fields are prone to be set incorrectly, and we recommend to enforce validation for trivial cases.

Remediation

The following checks were added to the TON and USDT pool contracts, respectively in commits 68b5af↗ and 0d6693↗:

• fee checks for UpdateBaseConfig, FeedPrices, WithdrawFee (for the USDT pool), ClaimProtocolFee (for the TON pool), requiring a minimum ctx.value

• require that FeedPrices messages contain prices for all configured tokens

  • note: FeedPrices could still contain prices for non-configured tokens

• check tpPrice and slPrice consistency when placing or modifying a perpetual order