sherlock-audit / 2024-02-tapioca-judging

2 stars 2 forks source link

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

Closed sherlock-admin4 closed 7 months ago

sherlock-admin4 commented 7 months ago

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 commented 7 months ago

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