sherlock-audit / 2024-02-tapioca-judging

3 stars 2 forks source link

0xadrii - Leverage module’s buyCollateral() function will always fail due to wrong parameter when depositing into yieldbox #116

Closed sherlock-admin2 closed 4 months ago

sherlock-admin2 commented 4 months ago

0xadrii

high

Leverage module’s buyCollateral() function will always fail due to wrong parameter when depositing into yieldbox

Summary

Passing a wrong parameter when depositing into YieldBox will make calls to buyCollateral() always fail.

Vulnerability Detail

The buyCollateral() feature in the BigBang and Singularity’s leverage module allows users to borrow more USDO and immediately set it as collateral. The summarized steps performed insidebuyCollateral() are:

  1. Withdraw USDO deposited from caller in YieldBox and transfer the USDO to the leverage executor
  2. Borrow USDO (by default deposited in YieldBox), withdraw it from YieldBox and transfer it to the leverage executor
  3. Swap the USDO for the collateral asset via the leverage executor
  4. Deposit the collateral into YieldBox (this is the wrong step detailed in this vulnerability).
  5. Trigger _addCollateral() so that the deposited YieldBox collateral assets are used as actual collateral in USDO

The problem with this vulnerability relies in step 4. As we can see in the following code snippet, after swapping the USDO for the collateral asset, the collateral is deposited into YieldBox. The problem is that the yieldBox.depositAsset() specifies that the shares obtained after depositing into YieldBox will be received by address(this) (the BBLeverage/SGLLeverage contract), instead of the calldata_.from address. Because the leverage contract is the one receiving the YieldBox shares, the final _addCollateral() call will fail (this call tries to pull the YieldBox share tokens from the user (calldata_.from), but the leverage module contract is the actual address that has the shares).

// BBLeverage.sol (also applies to SGLLeverage.sol)

function buyCollateral(address from, uint256 borrowAmount, uint256 supplyAmount, bytes calldata data) 
        external
        optionNotPaused(PauseType.LeverageBuy)
        solvent(from, false)
        notSelf(from)  
        returns (uint256 amountOut) 
    { 
        ...
        uint256 collateralShare = yieldBox.toShare(collateralId, amountOut, false);
        address(asset).safeApprove(address(yieldBox), type(uint256).max); 

        yieldBox.depositAsset(collateralId, address(this), address(this), 0, collateralShare); 
        address(asset).safeApprove(address(yieldBox), 0); 

         if (collateralShare == 0) revert CollateralShareNotValid();
        _allowedBorrow(calldata_.from, collateralShare);
        _addCollateral(calldata_.from, calldata_.from, false, 0, collateralShare); 
    } 

This will make buyCollateral() always fail because calldata_.from will never receive the YieldBox shares and a lack of balance will make the collateral addition always revert.

Impact

High. buyCollateral() will always fail and this feature will be unusable.

Code Snippet

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/Tapioca-bar/contracts/markets/singularity/SGLLeverage.sol#L88

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

Tool used

Manual Review

Recommendation

It is recommended to set the calldata_.from to be the receiver of the YieldBox deposit shares, instead of address(this):

// BBLeverage.sol (also applies to SGLLeverage.sol)

function buyCollateral(address from, uint256 borrowAmount, uint256 supplyAmount, bytes calldata data) 
        external
        optionNotPaused(PauseType.LeverageBuy)
        solvent(from, false)
        notSelf(from)  
        returns (uint256 amountOut) 
    { 
        ...
        uint256 collateralShare = yieldBox.toShare(collateralId, amountOut, false);
        address(asset).safeApprove(address(yieldBox), type(uint256).max); 

-        yieldBox.depositAsset(collateralId, address(this), address(this), 0, collateralShare); 
+        yieldBox.depositAsset(collateralId, address(this), calldata_.from, 0, collateralShare); 
        address(asset).safeApprove(address(yieldBox), 0); 

         if (collateralShare == 0) revert CollateralShareNotValid();
        _allowedBorrow(calldata_.from, collateralShare);
        _addCollateral(calldata_.from, calldata_.from, false, 0, collateralShare); 
    }

Duplicate of #100

maarcweiss commented 4 months ago

Dup of https://github.com/sherlock-audit/2024-02-tapioca-judging/issues/100

cryptotechmaker commented 4 months ago

Duplicate of https://github.com/sherlock-audit/2024-02-tapioca-judging/issues/57