GalloDaSballo / Apollon-Review

Notes for the Apollon Solo Security Review
0 stars 0 forks source link

`SwapOperations.addLiquidity` is not validating desired and min amounts #32

Open GalloDaSballo opened 3 months ago

GalloDaSballo commented 3 months ago

Impact

addLiquidity bases it's logic on the implicit invariant that amountDesired are greater than minAmounts

https://github.com/blkswnStudio/ap/blob/8fab2b32b4f55efd92819bd1d0da9bed4b339e87/packages/contracts/contracts/SwapOperations.sol#L242-L258

    {
      (vars.reserveA, vars.reserveB) = getReserves(tokenA, tokenB);
      if (vars.reserveA == 0 && vars.reserveB == 0) {
        (amountA, amountB) = (amountADesired, amountBDesired); /// @audit First LP Can attack by massively imbalancing by performing a single sided swap or similar
      } else {
        uint amountBOptimal = quote(amountADesired, vars.reserveA, vars.reserveB);
        if (amountBOptimal <= amountBDesired) {
          if (amountBOptimal < amountBMin) revert InsufficientBAmount();
          (amountA, amountB) = (amountADesired, amountBOptimal);
        } else {
          uint amountAOptimal = quote(amountBDesired, vars.reserveB, vars.reserveA);
          assert(amountAOptimal <= amountADesired); /// @audit Looks wrong
          if (amountAOptimal < amountAMin) revert InsufficientAAmount();
          (amountA, amountB) = (amountAOptimal, amountBDesired);
        }
      }
    }

However this check is missing from addLiquidity

Mitigation

Add the following checks

        if (amountADesired < amountAMin) revert InsufficientAmountADesired();
        if (amountBDesired < amountBMin) revert InsufficientAmountBDesired();

Also see: Velodrome Router

sambP commented 3 months ago
Screenshot 2024-08-28 at 10 33 52 PM