code-423n4 / 2023-12-particle-findings

2 stars 1 forks source link

Position cannot be liquidated if the borrower is blocklisted #16

Closed c4-bot-3 closed 11 months ago

c4-bot-3 commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-12-particle/blob/a3af40839b24aa13f5764d4f84933dbfa8bc8134/contracts/protocol/ParticlePositionManager.sol#L305 https://github.com/code-423n4/2023-12-particle/blob/a3af40839b24aa13f5764d4f84933dbfa8bc8134/contracts/protocol/ParticlePositionManager.sol#L456 https://github.com/code-423n4/2023-12-particle/blob/a3af40839b24aa13f5764d4f84933dbfa8bc8134/contracts/protocol/ParticlePositionManager.sol#L374

Vulnerability details

Impact

position cannot be liquidated if the borrower is blocklisted

Proof of Concept

When the position is closed, the function _closePosition is called

// execute actual position closing
_closePosition(params, cache, lien, msg.sender);

in this case, the msg.sender is the borrower who open the position

the close position logic involves refund logic

  // must ensure enough amount is left to pay for interest first, then send gains and fund left to borrower
        ///@dev refundWithCheck ensures actual cannot be more than expected, since amount owed to LP is in actual,
        ///     it ensures (1) on the collateralFrom part of refund, tokenOwed is covered, and (2) on the amountReceived
        ///      part, received is no less than liquidity addback + token owed.

while this refund logic works ok when borrower intends to close his position

the same _closePosition runs, the codes tries to close the position for borrower

// if borrower is blocklisted, cannot liquidate
_closePosition(params, closeCache, lien, borrower);

now we have a problem

https://github.com/d-xo/weird-erc20?tab=readme-ov-file#tokens-with-blocklists

Some tokens (e.g. USDC, USDT) have a contract level admin controlled address blocklist. If an address is blocked, then transfers to and from that address are forbidden.

Malicious or compromised token owners can trap funds in a contract by adding the contract address to the blocklist. This could potentially be the result of regulatory action against the contract itself, against a single user of the contract (e.g. a Uniswap LP), or could also be a part of an extortion attempt against users of the blocked contract.

assume the borrower who open the position is not blocklisted, so he can borrow the liquidity out

then he transfer the liquidity (borrowed fund) to another account and make trade

then the original borrower account is blocklisted

the liquiation will revert in refund flow when the code attempts to send the token to the blocklisted borrower

  function refund(address recipient, address token, uint256 amountExpected, uint256 amountActual) internal {
        if (amountExpected > amountActual) {
            TransferHelper.safeTransfer(token, recipient, amountExpected - amountActual);
        }
    }

    /**
     * @notice Helper function to refund a token, with additional check that amountActual must not exceed amountExpected
     * @param recipient the address to receive the refund
     * @param token address of token to potentially refund
     * @param amountExpected amount of token0 expected to spend
     * @param amountActual amount of token0 actually spent
     */
    function refundWithCheck(address recipient, address token, uint256 amountExpected, uint256 amountActual) internal {
        if (amountActual > amountExpected) revert Errors.OverRefund();
        refund(recipient, token, amountExpected, amountActual);
    }

Failed to liquidate the position make original lender lose fund

Tools Used

Manual Review

Recommended Mitigation Steps

let the original borrower claim the fund if their position are liquidated and avoid send token out during the liquidation flow

Assessed type

Token-Transfer

c4-judge commented 11 months ago

0xleastwood marked the issue as duplicate of #31

c4-judge commented 11 months ago

0xleastwood marked the issue as satisfactory