Approval for pearlmit is missing before repaying in the `depositRepayAndRemoveCollateralFromMarket` function of MagnetarAssetModule

[c4-bot-6]

Lines of code

https://github.com/Tapioca-DAO/tapioca-periph/blob/032396f701be935b04a7e5cf3cb40a0136259dbc/contracts/Magnetar/modules/MagnetarAssetModule.sol#L99-L106

Vulnerability details

Description

depositRepayAndRemoveCollateralFromMarket function attempts to repay the user's loan in the market. In the case of data.depositAmount > 0, it will use the assets existing in the contract (which was deposited by user before) to repay.

// @dev performs a repay operation for the specified market
if (data.repayAmount > 0) {
    _setApprovalForYieldBox(data.market, _yieldBox);
    (Module[] memory modules, bytes[] memory calls) = IMarketHelper(data.marketHelper).repay(
        data.depositAmount > 0 ? address(this) : data.user, data.user, false, data.repayAmount
    );
    _market.execute(modules, calls, true);
    _revertYieldBoxApproval(data.market, _yieldBox);
}

However, the Singularity.repay() function will pull YieldBox shares of the asset by using the pearlmit contract. Here is the code snippet of Singularity in the tapioca-bar gitmodule.

///https://github.com/Tapioca-DAO/Tapioca-bar/blob/0dca4a0b8e31b7aa59cb20540079b80a95bbe494/contracts/markets/singularity/SGLLendingCommon.sol
function _repay(address from, address to, bool skim, uint256 part) internal returns (uint256 amount) {
    ...

    uint256 share = yieldBox.toShare(assetId, amount, true);
    uint128 totalShare = totalAsset.elastic;
    _addTokens(from, to, assetId, share, uint256(totalShare), skim);
    ...
}

///https://github.com/Tapioca-DAO/Tapioca-bar/blob/0dca4a0b8e31b7aa59cb20540079b80a95bbe494/contracts/markets/singularity/SGLCommon.sol#L165-L177
function _addTokens(address from, address, uint256 _assetId, uint256 share, uint256 total, bool skim) internal {
    if (skim) {
        if (share > yieldBox.balanceOf(address(this), _assetId) - total) {
            revert TooMuch();
        }
    } else {
        // yieldBox.transfer(from, address(this), _assetId, share);
        bool isErr = pearlmit.transferFromERC1155(from, address(this), address(yieldBox), _assetId, share);
        if (isErr) {
            revert TransferFailed();
        }
    }
}

Because the depositRepayAndRemoveCollateralFromMarket function doesn't approve YieldBox's shares of the asset for the pearlmit contract of the market, pearlmit will be unable to pull shares from this contract. Therefore, it will revert when repaying in a Singularity market

Impact

depositRepayAndRemoveCollateralFromMarket will be broken

Tools Used

Manual review

Recommended Mitigation Steps

Should approve YieldBox shares of asset for pearlmit before repaying.

Assessed type

Other

[c4-sponsor]

cryptotechmaker (sponsor) confirmed

[c4-judge]

dmvt marked the issue as primary issue

[c4-judge]

dmvt marked the issue as duplicate of #125

[cryptotechmaker]

Fixed by https://github.com/Tapioca-DAO/tapioca-periph/commit/0a03bbbd04b30bcac183f1bae24d7f9fe9fd4103

[c4-judge]

dmvt changed the severity to 3 (High Risk)

[c4-judge]

dmvt marked the issue as satisfactory