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

2 stars 1 forks source link

_updateFundingRate() with hardcoded 1 days may not fit for all the scenario. #224

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-polynomial/blob/aeecafc8aaceab1ebeb94117459946032ccdff1e/src/Exchange.sol#L409-L424

Vulnerability details

Impact

This may update funding rate which would unfair to traders at certain sitation.

  1. when there are more number of trades happen in 24 hours period.
  2. when there are hardly few (in worst case no trade at all) trades happen.

Proof of Concept

Whenevenr trade is opened, closed, collateral added or removed, liquidated, _updateFundingRate is called.

This will update the fundingLastUpdated, normalizationFactor based on the fundingRate = fundingRate / 1 days;

Refer the code,

  function _updateFundingRate() internal {
    (int256 fundingRate,) = getFundingRate();
    fundingRate = fundingRate / 1 days;

    int256 currentTimeStamp = int256(block.timestamp);
    int256 fundingLastUpdatedTimestamp = int256(fundingLastUpdated);

    int256 totalFunding = wadMul(fundingRate, (currentTimeStamp - fundingLastUpdatedTimestamp));
    int256 normalizationUpdate = 1e18 - totalFunding;

    normalizationFactor = normalizationFactor.mulWadDown(uint256(normalizationUpdate));

    emit UpdateFundingRate(fundingLastUpdated, normalizationFactor);

    fundingLastUpdated = block.timestamp;
}

This normalizationFactor is used in getIndexPrice() to get the indexPrice which is used in getFundingRate()

This getFundingRate() is called getMarkPrice() to get the max price at the given trading scenario either open or close etc..

If there are less number of tades the price woudl exaggerated or more number of trades, the updated prices could not be optimunm.

Tools Used

Manual review

Recommended Mitigation Steps

We suggest to update the number of trades to update the funding rate.

function _updateFundingRate() internal { (int256 fundingRate,) = getFundingRate(); fundingRate = fundingRate / 1 days; ------------> change the hardocdes days .. based on number of trades

c4-sponsor commented 1 year ago

mubaris marked the issue as sponsor disputed

mubaris commented 1 year ago

getFundingRate() returns the 24 hrs funding rate and you want to change this?????

c4-sponsor commented 1 year ago

mubaris requested judge review

JustDravee commented 1 year ago

Vague arguments were given (could not be optimunm) for a heavy change. No numbers, no side by side comparison, no coded POC.

Couldn't find validity in this and the sponsor is obviously displeased. Closing as invalid.

c4-judge commented 1 year ago

JustDravee marked the issue as unsatisfactory: Invalid