balancer / balancer-core

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

Pools with a large total supply causes SWAP functions to always revert #209

Closed ggrieco-tob closed 4 years ago

ggrieco-tob commented 4 years ago

Severity: Low Difficulty: High

Description

A revert caused during the computations performed of SWAP functions can stop users to call these functions, if the initial supply of the token is large.

When a pool is finalized, the initial supply of shares is created. There is a lower bound to the initial supply, but there is no upper bound:

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

The total supply is used in several places, for instance in the joinswapExternAmountIn function which calls calcPoolOutGivenSingleIn:

https://github.com/balancer-labs/balancer-core/blob/942a51e202cc5bf9158bad77162bc72aa0a8afaf/contracts/BMath.sol#L142

The multiplications are done through the fixed-point arithmetic bmul:

https://github.com/balancer-labs/balancer-core/blob/942a51e202cc5bf9158bad77162bc72aa0a8afaf/contracts/BNum.sol#L63-L73

An overflow in this computation will revert, regardless of the input values used in the SWAP functions.

Exploit Scenario

Bob creates a pool with a very large number of token shares. Alice will try to call a SWAP function, but will only revert regardless of the input values she is using. As a result of that, she will be unable to use the pool as expected.

Recommendation

Short term:

Long term:

mikemcdonald commented 4 years ago

Fixed. Pools are not initialized with a constant INIT_POOL_SUPPLY of BONE * 100

https://github.com/balancer-labs/balancer-core/blob/master/contracts/BPool.sol#L236-L237