cergyk - BaseLeverageExecutor::_swapAndTransferToSender will return wrong amount if TOFT wrapping has fees

[sherlock-admin4]

cergyk

medium

BaseLeverageExecutor::_swapAndTransferToSender will return wrong amount if TOFT wrapping has fees

Summary

BaseLeverageExecutor::_swapAndTransferToSender does not account for fees which can be incurred when wrapping a TOFT. This will cause some calls in LeverageExecutors to systematically revert.

Vulnerability Detail

BaseLeverageExecutor::_swapAndTransferToSender implicitely considers that when wrapping, we get 1:1 TOFT token with regards to underlying:
https://github.com/sherlock-audit/2024-02-tapioca/blob/main/Tapioca-bar/contracts/markets/leverage/BaseLeverageExecutor.sol#L154

It returns amountOut which is also the input to _handleToftWrapToSender.

However we can see that wrapping can incur some fees in some cases:
https://github.com/sherlock-audit/2024-02-tapioca/blob/main/TapiocaZ/contracts/tOFT/mTOFT.sol#L300

This will cause BBLeverage/SGLLeverage functions buyCollateral and sellCollateral to revert due to insufficient balance

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/Tapioca-bar/contracts/markets/leverage/SimpleLeverageExecutor.sol#L60

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/Tapioca-bar/contracts/markets/bigBang/BBLeverage.sol#L144-L149

Impact

buyCollateral and sellCollateral will always revert for TOFT tokens having a minting fee

Code Snippet

Tool used

Manual Review

Recommendation

In _swapAndTransferToSender, one should get the actual amount obtained from wrapping:

    if (swapData.toftInfo.isTokenOutToft) {
-       _handleToftWrapToSender(sendBack, tokenOut, amountOut);
+       uint balanceBefore = IERC20(tokenOut).balanceOf(address(this));
+       _handleToftWrapToSender(sendBack, tokenOut, amountOut);
+       uint balanceAfter = IERC20(tokenOut).balanceOf(address(this));
+       amountOut = balanceBefore - balanceAfter;
    } else if (sendBack == true) {

Duplicate of #126

[sherlock-admin3]

1 comment(s) were left on this issue during the judging contest.

WangAudit commented:

in general; yest; it points the issue; but the report is flawed and just saying due to fees the txn will revert is not enough and it lacks details of how taking fees actually makes the transaction to revert