`usedFunds` is updated incorrectly when opening or closing any position in LiquidityPool

[code423n4]

Lines of code

https://github.com/code-423n4/2023-03-polynomial/blob/main/src/LiquidityPool.sol#L452 https://github.com/code-423n4/2023-03-polynomial/blob/main/src/LiquidityPool.sol#L484 https://github.com/code-423n4/2023-03-polynomial/blob/main/src/LiquidityPool.sol#L516 https://github.com/code-423n4/2023-03-polynomial/blob/main/src/LiquidityPool.sol#L549 https://github.com/code-423n4/2023-03-polynomial/blob/main/src/LiquidityPool.sol#L562

Vulnerability details

Impact

usedFunds of LiquidityPool is calculated incorrectly

Proof of concept

usedFunds tracks the funds utilitized from the pool. Let's see how usedFunds is accumulated when opening a long position:

function openLong(uint256 amount, address user, bytes32 referralCode)
    external
    override
    onlyExchange
    nonReentrant
    returns (uint256 totalCost)
{
    (uint256 markPrice, bool isInvalid) = getMarkPrice();
    require(!isInvalid);

    uint256 tradeCost = amount.mulWadDown(markPrice);
    uint256 fees = orderFee(int256(amount));
    totalCost = tradeCost + fees;

    SUSD.safeTransferFrom(user, address(this), totalCost);

    uint256 hedgingFees = _hedge(int256(amount), false);
    uint256 feesCollected = fees - hedgingFees;
    uint256 externalFee = feesCollected.mulWadDown(devFee);

    SUSD.safeTransfer(feeReceipient, externalFee);

    usedFunds -= int256(tradeCost);
    totalFunds += feesCollected - externalFee;

    emit RegisterTrade(referralCode, feesCollected, externalFee);
    emit OpenLong(markPrice, amount, fees);
}

usedFunds is still updated in function _hedge:

function _hedge(int256 size, bool isLiquidation) internal returns (uint256 hedgingFees) {
    bool isInvalid;

    uint256 delta = _getDelta();
    int256 hedgingSize = wadMul(size, int256(delta));

    (hedgingFees, isInvalid) = perpMarket.orderFee(hedgingSize, IPerpsV2MarketBaseTypes.OrderType.Delayed);
    require(!isInvalid);

    uint256 marginRequired = _calculateMargin(hedgingSize) + hedgingFees;
    usedFunds += int256(marginRequired);
    require(usedFunds <= 0 || totalFunds >= uint256(usedFunds));

    perpMarket.transferMargin(int256(marginRequired));

    emit TransferMargin(marginRequired);

    _placeDelayedOrder(hedgingSize, isLiquidation);
}

1. marginRequired in function _hedge includes the hedgingFees. Then usedFunds is updated correctly in function _hedge, that it increases the amount of SUSD tokens which is sent to PerpMarket.
2. In function openLong, the pool claims totalCost of SUSD tokens from user. And it only transfer externalFee outside of contract. Then usedFunds should be updated in function openLong as the following:

   usedFunds -= int256(totalCost) - externalFee;

Similarly, all functions closeLong, openShort, closeShort and liquidate update usedFunds incorrectly. It shoule be updated as the following:

• Function closeLong:

  usedFunds += int256(totalCost) + externalFee;

• Function openShort:

  usedFunds += int256(totalCost) + externalFee;

• Function closeShort:

  usedFunds -= int256(totalCost) - externalFee;

• Function liquidate: remove usedFunds += int256(hedgingFees);

  Tool used

Manual Review

Recommended Mitigation Steps

Should follow the above mitigation to fix the calculation of usedFunds

[c4-judge]

JustDravee marked the issue as duplicate of #152

[c4-judge]

JustDravee marked the issue as satisfactory

[c4-judge]

JustDravee changed the severity to 3 (High Risk)