By raising interest rates, some xChainCalls that use `repay ` can be denied

[c4-bot-8]

Lines of code

https://github.com/Tapioca-DAO/tapioca-periph/blob/2ddbcb1cde03b548e13421b2dba66435d2ac8eb5/contracts/Magnetar/modules/MagnetarOptionModule.sol#L169-L208

Vulnerability details

Impact

Since Magnetar performs xChain Operations, that can take some (small) amount of time

For calls that expect to repay 100% of the debt and withdraw their collateral

A front-runner can quickly borrow as much asset as they can on BB to raise the interest rate

This will cause the repay request to fail as the repayAmount is not an amount but rather a part, shares of the total debt

Meaning that the caller may not have sufficient credit to repay

https://github.com/Tapioca-DAO/tapioca-periph/blob/2ddbcb1cde03b548e13421b2dba66435d2ac8eb5/contracts/Magnetar/modules/MagnetarOptionModule.sol#L169-L208

// performs a BigBang repay operation
        if (!data.removeAndRepayData.assetWithdrawData.withdraw && data.removeAndRepayData.repayAssetOnBB) {
            _setApprovalForYieldBox(address(bigBang_), yieldBox_);

            (Module[] memory modules, bytes[] memory calls) = IMarketHelper(data.externalData.marketHelper).repay(
                address(this), data.user, false, data.removeAndRepayData.repayAmount  /// TODO @audit shares as amount
            );
            (bool[] memory successes, bytes[] memory results) = bigBang_.execute(modules, calls, true);

            if (!successes[0]) revert Magnetar_MarketCallFailed(calls[0]);
            uint256 repayed = IMarketHelper(data.externalData.marketHelper).repayView(results[0]);
            // transfer excess amount to the data.user
            if (repayed < _removeAmount) {
                uint256 bbAssetId = bigBang_.assetId();
                yieldBox_.transfer(
                    address(this), data.user, bbAssetId, yieldBox_.toShare(bbAssetId, _removeAmount - repayed, false)
                );
            }
        }

        // performs a BigBang removeCollateral operation
        // if `data.removeAndRepayData.collateralWithdrawData.withdraw` withdraws by using the `withdrawTo` method
        if (data.removeAndRepayData.removeCollateralFromBB) {
            uint256 _collateralId = bigBang_.collateralId();
            uint256 collateralShare = yieldBox_.toShare(_collateralId, data.removeAndRepayData.collateralAmount, false);
            address removeCollateralTo =
                data.removeAndRepayData.collateralWithdrawData.withdraw ? address(this) : data.user;

            (Module[] memory modules, bytes[] memory calls) = IMarketHelper(data.externalData.marketHelper)
                .removeCollateral(data.user, removeCollateralTo, collateralShare);
            bigBang_.execute(modules, calls, true);

            //withdraw
            if (data.removeAndRepayData.collateralWithdrawData.withdraw) {
                uint256 computedAmount = yieldBox_.toAmount(_collateralId, collateralShare, false);
                data.removeAndRepayData.collateralWithdrawData.lzSendParams.sendParam.amountLD = computedAmount;
                data.removeAndRepayData.collateralWithdrawData.lzSendParams.sendParam.minAmountLD = computedAmount;
                _withdrawToChain(data.removeAndRepayData.collateralWithdrawData);
            }
        }

POC

• User A calls for a repay of 100% of their debt
• User B sees the call, they quickly borrow a bunch of USDO on the same SGL and raise the interest rate
• User As ComposeCall will have to pay for the extra interest
• User A doesn't have sufficient funds
• User As call reverts

Mitigation

It's worth investigating whether a full repayment is always possible, as interest rates could cause debt to grow

Overall changing the logic to repay to be in asset amounts, and making end users aware of this risk should be sufficient

Assessed type

MEV

[c4-sponsor]

cryptotechmaker marked the issue as disagree with severity

[cryptotechmaker]

Informational

[c4-judge]

dmvt marked the issue as primary issue

[c4-sponsor]

0xRektora (sponsor) confirmed

[0xRektora]

I agree with @cryptotechmaker. Although that'd sometime kill the functionality, there's no harm being done. Position repayment can still be done on the same chain. Attacker has no incentives to do so.

[GalloDaSballo]

This finding is the written POC version of #154 The root cause is the same which is that repayAmount is part instead of elastic

[trungore]

@GalloDaSballo I believe this issue isn't related to issue #154, since this problem represents the manipulation of interest rates when repaying 100% of the debt. It's similar to a recommendation of slippage check for repayment. While #154 is about the missing conversion from shares to part of debt when calling repay(), which leads to incorrectly using the user's funds. The code snippet and mitigation are different, you should check it carefully.

[c4-judge]

dmvt changed the severity to QA (Quality Assurance)

[c4-judge]

dmvt marked the issue as grade-a

[cryptotechmaker]

Acknowledge