code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-polynomial/blob/aeecafc8aaceab1ebeb94117459946032ccdff1e/src/LiquidityPool.sol#L340-L365

Vulnerability details

Impact

LiquidityPool.getTokenPrice can revert.

Proof of Concept

In the implementation of LiquidityPool.getTokenPrice, totalValue is updated as follows.

    totalValue -= uint256((int256(amountOwed) + usedFunds));

When there are long positions, usedFunds will be negative and amountOwed can be less than -usedFunds when the base asset price goes down. So int256(amountOwed) + usedFunds will be less than 0 and it will be a huge value after converted to uint256. As a result, getTokenPrice will revert.

    uint256 amountOwed = markPrice.mulWadDown(powerPerp.totalSupply());

Tools Used

Manual Review

Recommended Mitigation Steps

We can add validation int256(amountOwed) + usedFunds >= 0 to prevent underflow.

JustDravee commented 1 year ago

Not sure if valid given that the tainted path isn't given/provided and is only hypothetical here. Issue would be valid if warden can provide a tainted path to get this state

c4-judge commented 1 year ago

JustDravee marked the issue as primary issue

mubaris commented 1 year ago

AFAIK this is not possible and if you think it's possible, please provide an scenario

c4-sponsor commented 1 year ago

mubaris marked the issue as sponsor disputed

JustDravee commented 1 year ago

Open to a coded POC during Post-Judging QA, otherwise this'll be considered as a hand-waved hypothesis