executeLimitOrder() calls modifyLongOi() and modifyShortOi() with wrong position size

[code423n4]

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/Trading.sol#L513-L517

Vulnerability details

Impact

executeLimitOrder() calls modifyLongOi() and modifyShortOi() with wrong position size, causes the following state variables wrongly updated

// Position.sol
fundingDeltaPerSec
longOi
shortOi

// PairsContract.sol
_idToOi

Proof of Concept

The vulnerability points

function executeLimitOrder(
    uint _id, 
    PriceData calldata _priceData,
    bytes calldata _signature
) 
    external
{
    unchecked {
        //...
        if (trade.direction) {
            tradingExtension.modifyLongOi(trade.asset, trade.tigAsset, true,
                trade.margin*trade.leverage/1e18); // @audit should be (trade.margin - _fee)*trade.leverage/1e18
        } else {
            tradingExtension.modifyShortOi(trade.asset, trade.tigAsset, true,
                trade.margin*trade.leverage/1e18); // @audit should be (trade.margin - _fee)*trade.leverage/1e18
        }
        // ...
    }
}

Tools Used

VS Code

Recommended Mitigation Steps

Use margin after fee to calculate position.

[c4-judge]

GalloDaSballo marked the issue as duplicate of #576

[c4-judge]

GalloDaSballo marked the issue as satisfactory