balancer / balancer-core

Balancer on the EVM
GNU General Public License v3.0
333 stars 168 forks source link

The swap in and swap out ratios are not correctly enforced #212

Closed ggrieco-tob closed 4 years ago

ggrieco-tob commented 4 years ago

Severity: Low Difficulty: Low

Description

The limits on the ratios to swap in and swap out tokens are not always correctly enforced.

The balance documentation defines maximum ratios when performing swap in and swap out operations:

Maximum Swap In Ratio - 1/2
A maximum swap in ratio of 0.50 means an user can only swap in less than 50% of the current balance of tokenIn for a given pool

Maximum Swap Out Ratio - 1/3
A maximum swap out ratio of 1/3 means an user can only swap out less than 33.33% of the current balance of tokenOut for a given pool

To define these limits, there are two constants in the BConst contract:

https://github.com/balancer-labs/balancer-core/blob/942a51e202cc5bf9158bad77162bc72aa0a8afaf/contracts/BConst.sol#L39-L40

These limits are supposed to be enforced in swapExactAmountIn and swapExactAmountOut:

https://github.com/balancer-labs/balancer-core/blob/942a51e202cc5bf9158bad77162bc72aa0a8afaf/contracts/BPool.sol#L442 https://github.com/balancer-labs/balancer-core/blob/942a51e202cc5bf9158bad77162bc72aa0a8afaf/contracts/BPool.sol#L504

However, it seems to be still possible to swap over the limits, since the checks are performed directly using the token balance. These are taken directly from the token supplies and the constants are made using BONE, so the result is not as precise as expected.

Exploit Scenario

Bob creates a new pool and several users join. They review the documentation and note the swap limits. However, Eve is able to swap tokens these over the limits. The users observe the large swaps from Eve. As a result of that, they decide to exit the pool, since they believe their funds are not longer secure.

Recommendation

Short term, simplify the implementation of the ratio checks in the swap in and swap out operations using bdiv.

Long term • Review all the documentation regarding pool limits. • Consider using Echidna and Manticore to test that pool limits are always enforced.