duc - `SGLLiquidation._extractLiquidationFees()` function transfers fee shares directly to Penrose, resulting in these fees being stuck

[sherlock-admin4]

duc

high

SGLLiquidation._extractLiquidationFees() function transfers fee shares directly to Penrose, resulting in these fees being stuck

Summary

The Penrose contract collects fees from markets by pulling these fee shares from the markets, using Market.refreshPenroseFees function. However, SGLLiquidation._extractLiquidationFees() transfers fee shares directly (in YieldBox) to Penrose, and these fees will be stuck in the Penrose contract since there is no way to collect them.

Vulnerability Detail

SGLLiquidation._extractLiquidationFees() function transfers fee shares directly to Penrose:

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);
    }
    ...
}

However, the Penrose contract collects fees by pulling fee shares from markets (using market.refreshPenroseFees) and withdrawing the pulled shares in the same function.

Penrose._depositFeesToTwTap :

function _depositFeesToTwTap(IMarket market, ITwTap twTap) private {
    if (!isMarketRegistered[address(market)]) revert NotValid();

    uint256 feeShares = market.refreshPenroseFees();
    if (feeShares == 0) return;

    address _asset = market.asset();
    uint256 _assetId = market.assetId();
    yieldBox.withdraw(_assetId, address(this), address(this), 0, feeShares);

   ...
}

Singularity.refreshPenroseFees:

 function refreshPenroseFees() external onlyOwner returns (uint256 feeShares) {
    ...
    feeShares = _removeAsset(_feeTo, msg.sender, balanceOf[_feeTo]);
}

There is no way to collect the directly transferred fee shares, since there is no function to extract the existing YieldBox shares in Penrose.

Impact

Fees collected from liquidations in Singularity markets will be lost

Code Snippet

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

Tool used

Manual Review

Recommendation

Don't transfer the fee shares to Penrose in the _extractLiquidationFees function; keep them in Singularity so Penrose can still collect them.

Duplicate of #148