code-423n4 / 2022-02-tribe-turbo-findings

1 stars 0 forks source link

TurboRouter: deposit(), mint(), createSafeAndDeposit() and createSafeAndDepositAndBoost() functions do not work #17

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

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 ERC4626RouterBase contract. When the user calls the deposit, mint, createSafeAndDeposit and createSafeAndDepositAndBoost functions of the TurboRouter contract, the deposit and mint functions of the ERC4626RouterBase contract are called.

  function deposit(IERC4626 safe, address to, uint256 amount, uint256 minSharesOut)
      public
      payable
      override
      authenticate(address(safe))
      returns (uint256)
  {
      return super.deposit(safe, to, amount, minSharesOut);
  }
...
  function deposit(
      IERC4626 vault,
      address to,
      uint256 amount,
      uint256 minSharesOut
  ) public payable virtual override returns (uint256 sharesOut) {
      if ((sharesOut = vault.deposit(amount, to)) < minSharesOut) {
          revert MinAmountError();
      }
  }

The deposit and mint functions of the ERC4626RouterBase contract will call the deposit and mint functions of the TurboSafe contract. The TurboSafe contract inherits from the ERC4626 contract, that is, the deposit and mint functions of the ERC4626 contract will be called.

The deposit and mint functions of the ERC4626 contract will call the safeTransferFrom function. Since the caller is the TurboRouter contract, msg.sender will be the TurboRouter contract. And because the user calls the deposit, mint, createSafeAndDeposit, and createSafeAndDepositAndBoost functions of the TurboRouter contract without transferring tokens to the TurboRouter contract and approving the TurboSafe contract to use the tokens, the call will fail.

  function deposit(uint256 amount, address to) public virtual returns (uint256 shares) {
      // Check for rounding error since we round down in previewDeposit.
      require((shares = previewDeposit(amount)) != 0, "ZERO_SHARES");

      // Need to transfer before minting or ERC777s could reenter.
      asset.safeTransferFrom(msg.sender, address(this), amount);

      _mint(to, shares);

      emit Deposit(msg.sender, to, amount, shares);

      afterDeposit(amount, shares);
  }

Proof of Concept

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboRouter.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);