hats-finance / Velvet-Capital-0x0bb0c08fd9eeaf190064f4c66f11d18182961f77

Core smart contracts of Velvet Capital
Other
0 stars 1 forks source link

DepositBatch : user can not make use of `multiTokenSwapAndTransfer` function to deposit tokens to the portolio contract #87

Open hats-bug-reporter[bot] opened 3 weeks ago

hats-bug-reporter[bot] commented 3 weeks ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xe7e1fdbf608a60508e54e95b612ad21d78b70e5c7d0d3d8bacb7960a9d0883b1 Severity: medium

Description: Description\

As the final call to transfer the token from the caller to the vault depends on the transferFrom function, a normal user can not deposit the token using the function multiTokenSwapAndTransfer in DepositBatch contract.

multiTokenSwapAndTransfer calls the multiTokenDepositFor function with following parameters.

DepositBatch.sol#L67-L71

    IPortfolio(data._target).multiTokenDepositFor(
      user, -->> caller (msg.sender)
      depositAmounts,
      data._minMintAmount
    );

multiTokenDepositFor works based on the approval based mechanism. whoever calls the multiTokenDepositFor, they should self-approve the contract to spend tokens. Because, the final call to deposit the token is done like this.

management/VaultManager.sol#L480-L486

        if (depositAmount == 0) revert ErrorLibrary.AmountCannotBeZero();
        if (usePermit) {
          _transferToVaultWithPermit(_from, portfolioTokens[i], depositAmount);
        } else {
          _transferToVault(_from, portfolioTokens[i], depositAmount);
        }
      }

where the _from is the caller.

VaultManager.sol#L357-L363

  function _transferToVault(
    address _from,
    address _token,
    uint256 _depositAmount
  ) internal {
    TransferHelper.safeTransferFrom(_token, _from, vault, _depositAmount);
  }

since the caller(msg.sender) is the _from, in the above call, the caller should approve themselve to spend the token. This is unexpected.

Refer the following function call to know the who is caller.

multiTokenDepositFor - _multiTokenDeposit - _handleTokenTransfer - here

while the above call routine would work if deposit is made by directly calling the multiTokenDepositFor in the portfolio contract, but will not work for the call made through the DepositBatch

when the multiTokenDepositFor is called in DepositBatch, the msg.sender will be the DepositBatch contract.

DepositBatch - msg.sender approves the portfolio contract to spend token - this is not used anywhere.

Nowhere, the approval for DepositBatch by the DepositBatch contract is done.

Due to this, the final call to transfer token using the transferFrom will revert due to insufficinet approval,

Impact\

Token can not be deposited through the DepositBatch contract.

  1. Revised Code File (Optional)

DepositBatch contract should self-approve for itself to spend token when calling the tranferFrom function.

aktech297 commented 3 weeks ago

This is incorrect. pls ignore.