Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Liquidate process may be reverted because of USDC's blacklist #6

Closed
c4-bot-4 opened this issue Jun 10, 2024 · 1 comment
Closed

Liquidate process may be reverted because of USDC's blacklist #6

c4-bot-4 opened this issue Jun 10, 2024 · 1 comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-42 edited-by-warden 🤖_27_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality

Comments

@c4-bot-4
Copy link
Contributor

c4-bot-4 commented Jun 10, 2024

Lines of code

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

Vulnerability details

Impact

When one position is not safe, liquidators should liquidate this position. The liquidation process may be reverted because of USDC's blacklist. If unsafe positions cannot be liquidated, the protocol has to take more risk. This is unexpected.

Proof of Concept

When one position is not safe, liquidators can liquidate this position and transfer left margin to vault.recipient if there is any. The vulnerability is that the left margin cannot be transferred to vault.recipient if vault.recipient is in the blacklist of USDC.

What's more, vault owner can update this vault's vault.recipient via function updateRecepient. If the trader does not trade via perp market or gamma market and trade with predy pool directly, the trader can become the vault's owner. This means if the user does not want to be liquidated when there is some margin left even if the position is not safe, the user can update vault.recipient to one known blacklist address of USDC. This will avoid liquidated when there is some margin left. And the trader can update vault.recipient back to himself via updateRecepient when he wants to close his position.

    function liquidate(
        uint256 vaultId,
        uint256 closeRatio,
        GlobalDataLibrary.GlobalData storage globalData,
        bytes memory settlementData
    ) external returns (IPredyPool.TradeResult memory tradeResult) {
        ...
        uint256 sentMarginAmount = 0;
        // hasPosition means the position is empty.
        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);
                }
            }

USDC transfer function with blacklist.

    function transfer(address to, uint256 value)
        external
        override
        whenNotPaused
        notBlacklisted(msg.sender)
        notBlacklisted(to)
        returns (bool)
    {
        _transfer(msg.sender, to, value);
        return true;
    }

Tools Used

Manual

Recommended Mitigation Steps

Consider to left the margin into the pool, and add one new function to let the trader to claim the left margin.

Assessed type

DoS

@c4-bot-4 c4-bot-4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Jun 10, 2024
c4-bot-4 added a commit that referenced this issue Jun 10, 2024
@c4-bot-12 c4-bot-12 added the 🤖_27_group AI based duplicate group recommendation label Jun 14, 2024
@howlbot-integration howlbot-integration bot added sufficient quality report This report is of sufficient quality duplicate-48 labels Jun 17, 2024
@c4-judge
Copy link
Contributor

alex-ppg marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-42 edited-by-warden 🤖_27_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

4 participants