code-423n4 / 2023-03-polynomial-findings

2 stars 1 forks source link

Function `hedgePositions` is incorrect, because it missed the `queuedPerpSize` in the calculation #113

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-polynomial/blob/main/src/LiquidityPool.sol#L568-L587

Vulnerability details

Impact

Function hedgePositions is incorrect, leads to the hedging will not work as expected, and LiquidityPool can lose funds without expectation.

Proof of concept

Let's see function hedgePositions in LiquidtyPool contract:

function hedgePositions() external override requiresAuth nonReentrant {
    int256 currentPosition = _getTotalPerpPosition();
    int256 skew = _getSkew();
    uint256 delta = _getDelta();
    int256 requiredPosition = wadMul(skew, int256(delta));

    int256 newPosition = requiredPosition - currentPosition;
    int256 marginDelta = int256(_calculateMargin(newPosition));

    if (requiredPosition.abs() < currentPosition.abs()) {
        marginDelta = -marginDelta;
    }

    usedFunds += marginDelta;

    perpMarket.transferMargin(marginDelta);
    _placeDelayedOrder(newPosition, false);

    emit HedgePositions(currentPosition, requiredPosition, marginDelta);
}

currentPosition is the sum of: the current position size of Liquidity Pool in Synthetix and the delta size of the current delayed order which was submitted into Synthetix perp market.

///url = https://github.com/code-423n4/2023-03-polynomial/blob/main/src/LiquidityPool.sol#L738-L743
function _getTotalPerpPosition() internal view returns (int256 positionSize) {
    IPerpsV2MarketBaseTypes.Position memory position = perpMarket.positions(address(this));
    IPerpsV2MarketBaseTypes.DelayedOrder memory delayedOrder = perpMarket.delayedOrders(address(this));

    positionSize = position.size + delayedOrder.sizeDelta;
}

However, currentPosition missed the variable queuedPerpSize, is the total amount of pending size delta (waiting to be submitted). Then _placeDelayedOrder will be called with the wrong newPosition, leads to the position size of pool can get a large deviation. The hedging will not be safe anymore.

Scenerio:

Manual Review

Recommended Mitigation Steps

currentPosition should be _getTotalPerpPosition() + queuedPerpSize in function hedgePositions

c4-sponsor commented 1 year ago

mubaris marked the issue as sponsor confirmed

c4-judge commented 1 year ago

JustDravee marked the issue as satisfactory

c4-judge commented 1 year ago

JustDravee marked the issue as selected for report