ElasticSwap contest - gzeon's results

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

Handle

gzeon

Vulnerability details

Impact

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

        tokenBQty = (_tokenAQty * _tokenBReserveQty) / _tokenAReserveQty;

since _tokenAReserveQty > 0 the division is safe, we can do something like

  assembly{
      if iszero(eq(div(mul(_tokenAQty ,_tokenBReserveQty),_tokenAQty),_tokenBReserveQty)) {revert(0,0)}
      tokenBQty := div(mul(_tokenAQty ,_tokenBReserveQty),_tokenAReserveQty)
  }

or use the following function

// out = x * y unchecked{/} z
function fmul(uint256 x, uint256 y, uint256 z) internal pure returns(uint256 out){
  assembly{
      if iszero(eq(div(mul(x,y),x),y)) {revert(0,0)}
      out := div(mul(x,y),z)
  }
}

to save some gas

Handle

gzeon

Vulnerability details

Impact

swapBaseTokenForQuoteToken check for _baseTokenQty > 0 && _minQuoteTokenQty > 0, but the same check also exists next line in MathLib.calculateQuoteTokenQty

Proof of Concept

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

    function swapBaseTokenForQuoteToken(
        uint256 _baseTokenQty,
        uint256 _minQuoteTokenQty,
        uint256 _expirationTimestamp
    ) external nonReentrant() {
        isNotExpired(_expirationTimestamp);
        require(
            _baseTokenQty > 0 && _minQuoteTokenQty > 0,
            "Exchange: INSUFFICIENT_TOKEN_QTY"
        );

        uint256 quoteTokenQty =
            MathLib.calculateQuoteTokenQty(
                _baseTokenQty,
                _minQuoteTokenQty,
                TOTAL_LIQUIDITY_FEE,
                internalBalances
            );

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

    function calculateQuoteTokenQty(
        uint256 _baseTokenQty,
        uint256 _quoteTokenQtyMin,
        uint256 _liquidityFeeInBasisPoints,
        InternalBalances storage _internalBalances
    ) public returns (uint256 quoteTokenQty) {
        require(
            _baseTokenQty > 0 && _quoteTokenQtyMin > 0,
            "MathLib: INSUFFICIENT_TOKEN_QTY"
        );

Handle

gzeon

Vulnerability details

Impact

> 0 is less gas efficient than != 0 for uint in require condition when optimizer is enabled Ref: https://twitter.com/GalloDaSballo/status/1485430908165443590

Proof of Concept

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

        require(this.totalSupply() > 0, "Exchange: INSUFFICIENT_LIQUIDITY");

Handle

gzeon

Vulnerability details

Impact

Currently an Exchange is deployed by https://github.com/code-423n4/2022-01-elasticswap/blob/d107a198c0d10fbe254d69ffe5be3e40894ff078/elasticswap/src/contracts/ExchangeFactory.sol#L55

        Exchange exchange =
            new Exchange(
                _name,
                _symbol,
                _baseToken,
                _quoteToken,
                address(this)
            );

        exchangeAddressByTokenAddress[_baseToken][_quoteToken] = address(
            exchange
        );
        isValidExchangeAddress[address(exchange)] = true;

        emit NewExchange(msg.sender, address(exchange));

which will generate a non-deterministic contract address for the pair. When a contract want to find the Exchange address for a specific pair, they would have to lookup exchangeAddressByTokenAddress[][] costing a SLOAD. Instead, we can use CREATE2 to generate deterministic contract address for each pair.

bytes memory bytecode = type(Exchange).creationCode;
bytes32 salt = keccak256(abi.encodePacked(_baseToken, _quoteToken));
assembly {
    exchange := create2(0, add(bytecode, 32), mload(bytecode), salt)
}

To find the pair we can calculate the address, see https://github.com/sushiswap/sushiswap/blob/45da97206358039039883c4a99c005bb8a4bef0c/contracts/uniswapv2/libraries/UniswapV2Library.sol#L20

Handle

gzeon

Vulnerability details

Impact

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

        // We should ensure no possible overflow here.
        if (quoteTokenQtyToReturn > internalBalances.quoteTokenReserveQty) {
            internalBalances.quoteTokenReserveQty = 0;
        } else {
            internalBalances.quoteTokenReserveQty -= quoteTokenQtyToReturn;
        }

to

        // We should ensure no possible overflow here.
        if (quoteTokenQtyToReturn > internalBalances.quoteTokenReserveQty) {
            internalBalances.quoteTokenReserveQty = 0;
        } else {
            unchecked{
                internalBalances.quoteTokenReserveQty -= quoteTokenQtyToReturn;
            }
        }