GalloDaSballo / Apollon-Review

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

`SwapOperations` is performing some swaps without updating the price, resulting in incorrect fees being charged #30

Open GalloDaSballo opened 3 months ago

GalloDaSballo commented 3 months ago

Impact

Operations such as: swapExactTokensForTokens, swapTokensForExactTokens receive _priceUpdateData but update the price only at the _swap operation

_swap uses getAmountsOut and getAmountsIn to determine the price at which to charge fees at

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


  function swapExactTokensForTokens(
    uint amountIn,
    uint amountOutMin,
    address[] calldata path,
    address to,
    uint deadline,
    bytes[] memory _priceUpdateData
  ) public payable virtual override ensure(deadline) returns (SwapAmount[] memory amounts) {
    amounts = getAmountsOut(amountIn, path); /// @audit Used old prices
    if (amounts[amounts.length - 1].amount < amountOutMin) revert InsufficientOutputAmount();

    safeTransferFrom(path[0], msg.sender, getPair[path[0]][path[1]], amounts[0].amount);
    _swap(amounts, path, to, _priceUpdateData, false); /// @audit Updates prices after having used old prices
  }

This means that fees on swaps are computed on the stale price rather then the latest

Because oracle prices can be updated via other operations, this also allows swappers to pick the most optimal price at which to pay the lowest fee

Mitigation

You can ensure the price is updated by:

sambP commented 3 months ago
Screenshot 2024-08-27 at 5 47 06 PM

running the updates before getAmountsOut/In