sherlock-audit / 2023-07-perennial-judging

2 stars 1 forks source link

[Perennial Self Report] Fix non-requested commits after oracle grace period #177

Open arjun-io opened 1 year ago

arjun-io commented 1 year ago

Medium

When a requested version was unavailable, non-requested versions would be blocked from being to be committed until a new requested version was committed. This could prevent liquidations from occurring.

https://github.com/equilibria-xyz/perennial-v2/pull/58

hrishibhat commented 12 months ago

This issue is not included in the contest pool rewards

arjun-io commented 12 months ago

Fixed: https://github.com/equilibria-xyz/perennial-v2/pull/58

MLON33 commented 11 months ago

From WatchPug,

https://github.com/equilibria-xyz/perennial-v2/blob/f216b2fd0ec45ea22b443a00fdfe2257f803ce7c/packages/perennial-oracle/contracts/pyth/PythOracle.sol#L89-L102

Screenshot 2023-09-14 at 5 11 03 PM

https://github.com/equilibria-xyz/perennial-v2/blob/f216b2fd0ec45ea22b443a00fdfe2257f803ce7c/packages/perennial-oracle/contracts/pyth/PythOracle.sol#L125-L203

Screenshot 2023-09-14 at 5 11 23 PM Screenshot 2023-09-14 at 5 11 32 PM

latest() may unexpectedly go back in time.

Given:

versionList[18]: 18:59:58 versionList[19]: 19:00:00 nextVersionIndexToCommit: 11 When:

Recommendation

In addition to the invariant of newPrice.publishTime > lastCommittedPublishTime, make sure newLatestVersion > oldLatestVersion.

arjun-io commented 11 months ago

From WatchPug,

https://github.com/equilibria-xyz/perennial-v2/blob/f216b2fd0ec45ea22b443a00fdfe2257f803ce7c/packages/perennial-oracle/contracts/pyth/PythOracle.sol#L89-L102

Screenshot 2023-09-14 at 5 11 03 PM

https://github.com/equilibria-xyz/perennial-v2/blob/f216b2fd0ec45ea22b443a00fdfe2257f803ce7c/packages/perennial-oracle/contracts/pyth/PythOracle.sol#L125-L203

Screenshot 2023-09-14 at 5 11 23 PM Screenshot 2023-09-14 at 5 11 32 PM latest() may unexpectedly go back in time.

Given:

versionList[18]: 18:59:58 versionList[19]: 19:00:00 nextVersionIndexToCommit: 11 When:

  • commit({versionIndex: 19, oracleVersion: 18:59:59, updateData: (pyth data of 19:00:03)})

    • At L180, oracleVersion == versionList[versionIndex] i.e., 18:59:59 == 19:00:00 is false, so it won't enter commitRequested()
    • commit() does not have a check like commitRequested() L153-L157, so it can skip versionIndex: 18, even if updateData is a valid price for versionIndex: 18
  • At this point, latest() returns OracleVersion(timestamp: 18:59:59, ...)
  • commitRequested({versionIndex: 18, updateData: (pyth data of 19:00:04)})

    • commitRequested() does not have a check like commit() L199's newlatestVersion > oldlatestVersion (i.e., versionToCommit > _latestVersion)
  • At this point, latest() returns OracleVersion(timestamp: 18:59:58, ...)

Recommendation

In addition to the invariant of newPrice.publishTime > lastCommittedPublishTime, make sure newLatestVersion > oldLatestVersion.

We've oped to fix this by setting the nextVersionIndexToCommit in the unrequested commit flow

fix: https://github.com/equilibria-xyz/perennial-v2/pull/100