DepositBatch us not compatible with fee on transfer tokens

[hats-bug-reporter[bot]]

Github username: @aktech297 Twitter username: kaka Submission hash (on-chain): 0x60d96e8f99fadf43cf6461745bc9c8f90dcc27fa7a61310b73408a79523d0449 Severity: medium

Description: Description\ DepositBatch - multiTokenSwapAndTransfer function deposit tokens where it assumes all the tokens are tranferred from the user.

This is not true if we consider the some token which charge fee while transfer.

Attack Scenario\

when user transfer the token , though the actual token amount is lesser than the actual value, the contract assumes that all token are transferred from user. This would lead to unexpected issue during token transfer handling.

Attachments

1. Proof of Concept (PoC) File https://github.com/hats-finance/Velvet-Capital-0x0bb0c08fd9eeaf190064f4c66f11d18182961f77/blob/aa47c9ff85bcc2bede62978c3895668b549da125/contracts/bundle/DepositBatch.sol#L51-L65

    for (uint256 i; i < tokenLength; i++) {
      address _token = tokens[i];
      uint256 balance;
      if (_token == _depositToken) {
        //Sending encoded balance instead of swap calldata
        balance = abi.decode(data._callData[i], (uint256)); --- @@ check for this as example.
      } else {
        (bool success, ) = SWAP_TARGET.delegatecall(data._callData[i]);
        if (!success) revert ErrorLibrary.CallFailed();
        balance = _getTokenBalance(_token, address(this));
      }
      if (balance == 0) revert ErrorLibrary.InvalidBalanceDiff();
      IERC20(_token).approve(data._target, balance);
      depositAmounts[i] = balance;
    }

2. Revised Code File (Optional) we suggest to check the balance before and after token tranfer and calculated the actual transferred amount.

[langnavina97]

That's why we calculate the balances before and after the transfer in the vault manager. We are aware that sometimes values are lost during token transfers, such as with compound tokens. This approach ensures that we account for any discrepancies and handle the tokens correctly.

@aktech297

[aktech297]

Hi @langnavina97 ,

We analyzed the codes again and we can see issue due to fee on transfer. lets see in details.

We identified the following two issues with fee on transfer tokens.

1. Token transfer will revert here due to insufficient amount of tokens.
2. Incorrect calculation of mint ratio which will be used to mint the tokens to the user.

Lets see the first one.

In short about the DepositBatch contract,

• user deposit token to the DepositBatch contract
• swap or directly decode the amount into an array
• call the multiTokenDepositFor - tokens are transferred from the DepositBatch to vault manager.

multiTokenSwapAndTransfer - if the _depositToken token is FOT, the received amount is lesser than the data._depositAmount - here

But the depositAmounts array will have slightly higher than what was received. This array is passed to multiTokenDepositFor here.

The call flow will be multiTokenSwapAndTransfer - multiTokenDepositFor - _handleTokenTransfer.

Please note that the msg.sender will be the BatchDeposit contract in the _handleTokenTransfer. here the array is traversed and funds are sent to vault here. here the BatchDeposit will have the less balance than the array balance value for FOT. When trying to r transfer more value, the transaction will revert due to insufficient value.

Now, lets see the incorrect mint ratio calculation.

User can directly call the multiTokenDepositFor with array of deposit amounts, min mint amount and to whom the deposit wanted to be.

In function _handleTokenTransfer min ration is calculated using this array of balances. But if we see the FOT, these array of values will differ with the actual transferred amount. So the calculated ratio will be slightly higher than the actual value.

Based on this ration, mint amount is calculated and tokens are minted.