code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/vault/strategy/Pool2SingleAssetCompounder.sol#L62

Vulnerability details

Impact

Lack of slippage check when perform V2 Uniswap V2 trade in Pool2SingleAssetCompounder

Proof of Concept

In the current implementation of the code, the harvest function is called:

  /// @notice claim all token rewards and trade them for the underlying asset
  function harvest() public override {
    address router = abi.decode(IAdapter(address(this)).strategyConfig(), (address));
    address asset = IAdapter(address(this)).asset();
    address[] memory rewardTokens = IWithRewards(address(this)).rewardTokens();

    IWithRewards(address(this)).claim(); // hook to accrue/pull in rewards, if needed

    address[] memory tradePath = new address[](2);
    tradePath[1] = asset;

    uint256 len = rewardTokens.length;
    // send all tokens to destination
    for (uint256 i = 0; i < len; i++) {
      uint256 amount = ERC20(rewardTokens[i]).balanceOf(address(this));

      if (amount > 0) {
        tradePath[0] = rewardTokens[i];

        IUniswapRouterV2(router).swapExactTokensForTokens(amount, 0, tradePath, address(this), block.timestamp);
      }
    }
    IAdapter(address(this)).strategyDeposit(ERC20(asset).balanceOf(address(this)), 0);
  }

note the Uniswap V2 trade:

IUniswapRouterV2(router).swapExactTokensForTokens(amount, 0, tradePath, address(this), block.timestamp);

If we look into the API of the swapExactTokensForTokens in Uniswap V2,

https://docs.uniswap.org/contracts/v2/reference/smart-contracts/router-02#swapexacttokensfortokens

function swapExactTokensForTokens(
  uint amountIn,
  uint amountOutMin,
  address[] calldata path,
  address to,
  uint deadline
) external returns (uint[] memory amounts);

We can see that the amoutnOutMin slippage check parameter is missing, this makes the code vulnerable to frontrunning and sandwich attack, a sandwich bot can front-run the harvest function, buy before the IUniswapRouterV2(router).swapExactTokensForTokens trade to push the price up and after the trade sell and dump the token for profits at the loss of the reward token.

Tools Used

Manual Review

Recommended Mitigation Steps

We recommend the protocol implementatoin proper slippage check instead of set the amountOutMint to 0 when trade happens.

c4-sponsor commented 1 year ago

RedVeil marked the issue as sponsor disputed

c4-judge commented 1 year ago

dmvt marked the issue as unsatisfactory: Out of scope

code-423n4 / 2023-01-popcorn-findings