Full liquidation of vault will not be possible when there is a left over margin and the vault recipient is blacklisted.

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-05-predy/blob/main/src/libraries/logic/LiquidationLogic.sol#L89-L100

Vulnerability details

Impact

Full liquidation of vault will not be possible when there is a left over margin and the vault recipient is blacklisted.

Proof of Concept

// ...
library LiquidationLogic {
    // ...
    function liquidate(
        uint256 vaultId,
        uint256 closeRatio,
        GlobalDataLibrary.GlobalData storage globalData,
        bytes memory settlementData
    ) external returns (IPredyPool.TradeResult memory tradeResult) {
        if (!hasPosition) {
            int256 remainingMargin = vault.margin;

            if (remainingMargin > 0) {
                if (vault.recipient != address(0)) {
                    // Send the remaining margin to the recipient.
                    vault.margin = 0;

                    sentMarginAmount = uint256(remainingMargin);
                    
                  ERC20(pairStatus.quotePool.token).safeTransfer(vault.recipient, sentMarginAmount); // <=== Audit
                }
            } else if (remainingMargin < 0) {
                // ...
            }
        }
    }
}

At the end of a full liquidation process, any left over margin is sent to the vault recipient.

If the vault recipient is blacklisted by the token contract (for example USDC and USDT), the transfer will revert making the full liquidation of the vault impossible.

This can happen if :

• The vault recipient is blacklisted after interacting with the protocol.
• A pair doesn't have whitelist enabled and allows any traders. A trader can intentionally set the vault recipient for an owned vault to a blacklisted address after interacting with the protocol.

Tools Used

Manual Review

Recommended Mitigation Steps

Add a try/catch to the transfer call when sending the left over margin to the vault recipient.

Assessed type

Other

[c4-judge]

alex-ppg marked the issue as partial-75

[imsrybr0]

Hi @alex-ppg,

Can you please let me know why this was marked as Partial-75 ?

My understanding is that vaults created by markets (PerpMarket and GammaMarket for example) are owned by those markets, so it's not possible to directly update the vault recipient in that case and this issue can only happen if the recipient is blacklisted after interacting with them or if an allowed market provides the ability to update the vault recipient.

Thank you

[alex-ppg]

Hey @imsrybr0, thank you for your contribution! I have re-instated a full reward for this submission as I had misread the recommended mitigation steps.

[c4-judge]

alex-ppg marked the issue as satisfactory