TurboRouter: deposit(), mint(), createSafeAndDeposit() and createSafeAndDepositAndBoost() functions may be vulnerable to FRONT-RUN attack

[code423n4]

Lines of code

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboRouter.sol

Vulnerability details

Impact

The TurboRouter contract inherits from the PeripheryPayments contract. To execute the deposit(), mint(), createSafeAndDeposit() and createSafeAndDepositAndBoost() functions of the TurboRouter contract, the user needs to first execute the pullToken and approve functions of the PeripheryPayments contract to transfer tokens to the TurboRouter contract and approve the TurboSafe contract to use these tokens . But users may be exposed to FRONT-RUN attack between these three steps.

     function approve(ERC20 token, address to, uint256 amount) public payable {
        token.safeApprove(to, amount);
    }
    function pullToken(ERC20 token, uint256 amount, address recipient) public payable {
        token.safeTransferFrom(msg.sender, recipient, amount);
    }

1. After the user calls pullToken to transfer the tokens to the TurboRouter contract, the attacker can call the approve function to approve the use of these tokens by any contract
2. After the user calls the approve function to approve the TurboSafe contract to use these tokens, the attacker can use these tokens to execute the deposit(), mint(), createSafeAndDeposit() and createSafeAndDepositAndBoost() functions to profit

   Proof of Concept

   https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboRouter.sol

https://github.com/fei-protocol/ERC4626/blob/5b786fe0317f65f5b716f577c28092fa349c4903/src/external/PeripheryPayments.sol

Tools Used

None

Recommended Mitigation Steps

In the deposit, mint, createSafeAndDeposit, and createSafeAndDepositAndBoost functions of the TurboRouter contract, add code for the user to transfer tokens and approve the use of tokens in the TurboSafe contract. For example:

TurboRouter.sol

+        IERC20(safe.asset).safeTransferFrom(msg.sender,address(this),amount);
+        IERC20(safe.asset).safeApprove(safe,amount);
        super.deposit(IERC4626(address(safe)), to, amount, minSharesOut);

...

+        IERC20(safe.asset).safeTransferFrom(msg.sender,address(this),amount);
+        IERC20(safe.asset).safeApprove(safe,amount);
        super.mint(safe, to, shares, maxAmountIn);

[Joeysantoro]

Incorrect, these can be batched in a Multicall

[GalloDaSballo]

Have to side with the sponsor here, the router is meant to be called by other contracts making these type of exploits invalid