sherlock-audit / 2023-01-derby-judging

4 stars 1 forks source link

tives - MainVault.withdrawRewards and Vault.claimTokens make an UniV3 swap without slippage protection #354

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

tives

high

MainVault.withdrawRewards and Vault.claimTokens make an UniV3 swap without slippage protection

Summary

MainVault.withdrawRewards and Vault.claimTokens make an UniV3 swap without slippage protection

Vulnerability Detail

Both Vault.claimTokens and MainVault.withdrawRewards call Swap.swapTokensMulti. This function gets the amountOutMinimum quote from UniV3 via IQuoter as amountOutMinimum.

uint256 amountOutMinimum = IQuoter(_uniswap.quoter).quoteExactInput(
  abi.encodePacked(_swap.tokenIn, _uniswap.poolFee, WETH, _uniswap.poolFee, _swap.tokenOut),
  _swap.amount
);

This amountOutMinimum is then input to the actual swap as amountOutMinimum. This value is by all means the current price of the vault. There is no protection against flash loan price manipulation. Attacker can therefore observe the mempool and force a swap with very bad price

Attack

Impact

Project/user loses funds due to no slippage protection

Code Snippet

function withdrawRewards() external nonReentrant onlyWhenIdle returns (uint256 value) {
    ...
    if (swapRewards) {
      uint256 tokensReceived = Swap.swapTokensMulti(
        Swap.SwapInOut(value, address(vaultCurrency), derbyToken),
        controller.getUniswapParams(),
        true
      );
      IERC20(derbyToken).safeTransfer(msg.sender, tokensReceived);
    } else {
      vaultCurrency.safeTransfer(msg.sender, value);
    }
  }

https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/MainVault.sol/#L208

Tool used

Manual Review

Recommendation

Verify the swap price via an price oracle before doing the swap.

Add minOut parameter to claimTokens/withdrawRewards

Duplicate of #64