code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-polynomial/blob/aeecafc8aaceab1ebeb94117459946032ccdff1e/src/LiquidityPool.sol#L472-L484 https://github.com/code-423n4/2023-03-polynomial/blob/aeecafc8aaceab1ebeb94117459946032ccdff1e/src/LiquidityPool.sol#L807-L808 https://github.com/code-423n4/2023-03-polynomial/blob/aeecafc8aaceab1ebeb94117459946032ccdff1e/src/LiquidityPool.sol#L549 https://github.com/code-423n4/2023-03-polynomial/blob/aeecafc8aaceab1ebeb94117459946032ccdff1e/src/LiquidityPool.sol#L516

Vulnerability details

Impact

usedFunds is wrong in LiquidityPool, and usedFunds tracks spent quote tokens. usedFunds is an important state in LiquidityPool, so the impact will be high.

Proof of Concept

Liquidity.closeLong and openShort don't update the state usedFunds correctly.

In the implementation of closeLong, tradeCost is added to usedFunds.

    usedFunds += int256(tradeCost);

But tradeCost already contains hedgingFees and hedgingFees are added to usedFunds in _hedge method before.

        uint256 marginRequired = _calculateMargin(hedgingSize) + hedgingFees;
        usedFunds += int256(marginRequired);

So hedgingFees are added to usedFunds twice, and usedFunds will be wrong. There are similar things in openShort method, too. In the implementation of openShort, hedgingFees are added to usedFunds twice from direct addition and _hedge method similarly.

    usedFunds += int256(totalCost + hedgingFees + externalFee);

Tools Used

Manual Review

Recommended Mitigation Steps

we can use totalCost instead of tradeCost to update usedFunds as follows for closeLong. And same thing for openShort.

    usedFunds += int256(totalCost);

And this is for closeShort:

    usedFunds -= int256(tradeCost);

c4-judge commented 1 year ago

JustDravee marked the issue as duplicate of #152