Revert when K >= 2^256

[code423n4]

Handle

gzeon

Vulnerability details

Impact

The constant product k is calculated by baseTokenReserve*quoteTokenReserve, when this resolves to >= 2^256, some math in MathLib will overflow causing revert and all fund would be locked in the Exchange until the balance decrease. There exists no mechanism like skim() in UniswapV2 that remove excess token from the pool.

This is especially dangerous to elasticswap because rebase token can have their balance grow very quickly.

Proof of Concept

https://github.com/code-423n4/2022-01-elasticswap/blob/d107a198c0d10fbe254d69ffe5be3e40894ff078/elasticswap/src/libraries/MathLib.sol#L17

        uint256 kLast; // as of the last add / rem liquidity event

https://github.com/code-423n4/2022-01-elasticswap/blob/d107a198c0d10fbe254d69ffe5be3e40894ff078/elasticswap/src/contracts/Exchange.sol#L109

        internalBalances.kLast =
            internalBalances.baseTokenReserveQty *
            internalBalances.quoteTokenReserveQty;

https://github.com/code-423n4/2022-01-elasticswap/blob/d107a198c0d10fbe254d69ffe5be3e40894ff078/elasticswap/src/libraries/MathLib.sol#L150

        qtyToReturn =
            (tokenASwapQtyLessFee * _tokenBReserveQty) /
            ((_tokenAReserveQty * BASIS_POINTS) + tokenASwapQtyLessFee);

Recommended Mitigation Steps

Implement a skim() function to remove excess token in the pool or otherwise make sure removeLiquidity can be called even when baseTokenReserve*quoteTokenReserve overflow.

Currently removeLiquidity will revert when baseTokenReserve*quoteTokenReserve overflow due to the calculation of kLast here: https://github.com/code-423n4/2022-01-elasticswap/blob/d107a198c0d10fbe254d69ffe5be3e40894ff078/elasticswap/src/contracts/Exchange.sol#L231

        internalBalances.kLast =
            internalBalances.baseTokenReserveQty *
            internalBalances.quoteTokenReserveQty;

unless the user remove enough token to make it not overflow

[0xean]

Adding of liquidity cannot cause the overflow, since it will revert before the overflow would occur when calculating K.

Let's take an example also to see how large this would have to be to occur. Let's take a token that has a total supply of 10 billion and 18 decimals. If the total supply of two of these tokens was in our contracts we would end up with a K as below:

100000000000000000000000000000000000000000000000000000000 The max uint256 is: 115792089237316195423570985008687907853269984665640564039457584007913129639935

Uniswap v2 also doesn't use uint256 for their variables.

Would recommend low or medium risk for this.

[GalloDaSballo]

The warden is stating a fact, if the math goes above 2^256 it will revert.

The sponsor says that Adding of liquidity cannot cause the overflow that is correct, because the MathLib will calculate K and revert.

The sponsor example shows how it's incredibly unlikely that any token would cause a revert, I went and multiplied safemoons (9 decimals, trillions in circulation) and I still am 30 orders of magnitude away from a revert

I think the observation is valid, but ultimately extremely unlikely.

I agree with a low severity and don't believe the sponsor has to mitigate