sherlock-audit / 2024-02-tapioca-judging

2 stars 2 forks source link

bin2chen - SGL Liquidation Fees be Locked in Penrose #51

Closed sherlock-admin3 closed 5 months ago

sherlock-admin3 commented 5 months ago

bin2chen

high

SGL Liquidation Fees be Locked in Penrose

Summary

During the execution of SGL._extractLiquidationFees(), the LiquidationFees are directly stored in the penrose contract. However, the penrose contract does not offer any method to handle these fees already present within it.

Vulnerability Detail

When SGL undergoes liquidation, the _extractLiquidationFees() function stores the LiquidationFees in the penrose contract. The code implementation is as follows: _liquidateUser()->_extractLiquidationFees()

    function _extractLiquidationFees(uint256 extraShare, uint256 callerReward)
        private
        returns (uint256 feeShare, uint256 callerShare)
    {
        callerShare = (extraShare * callerReward) / FEE_PRECISION; //  y%  of profit goes to caller.
        feeShare = extraShare - callerShare; // rest goes to the fee

        if (feeShare > 0) {
            uint256 feeAmount = yieldBox.toAmount(assetId, feeShare, false);
@>          yieldBox.depositAsset(assetId, address(this), address(penrose), feeAmount, 0);
        }
        if (callerShare > 0) {
            uint256 callerAmount = yieldBox.toAmount(assetId, callerShare, false);
            yieldBox.depositAsset(assetId, address(this), msg.sender, callerAmount, 0);
        }
    }

However, the penrose contract lacks any method to process these fees that are already within it. Even the penrose.withdrawAllMarketFees() function cannot handle these fees already residing in the penrose contract. Consequently, this portion of fees remains locked within the penrose contract.

Impact

The SGL Liquidation Fees will remain locked in the penrose contract.

Code Snippet

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

Tool used

Manual Review

Recommendation

Recommendation: It is advisable to introduce a method to transfer this portion of fees to twTap

+    function distributeBalanceOnTwTap(address _asset,uint256 _assetId) external onlyOwner notPaused {
+       uint256 feeShares = yieldBox.balanceOf(address(this), _assetId);
+       (uint256 feeAmount ,_ ) = yieldBox.withdraw(_assetId, address(this), address(this), 0, feeShares);
+       uint256 rewardTokenId = twTap.rewardTokenIndex(_asset);
+       _distributeOnTwTap(feeAmount, rewardTokenId, _asset, twTap);
+    }

Duplicate of #148