Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

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

Open
arjun-io opened this issue Aug 18, 2023 · 4 comments
Labels
Medium A valid Medium severity issue Non-Reward This issue will not receive a payout Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@arjun-io
Copy link

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.

equilibria-xyz/perennial-v2#58

@arjun-io arjun-io added Medium A valid Medium severity issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed Non-Reward This issue will not receive a payout labels Aug 18, 2023
@141345 141345 closed this as completed Aug 23, 2023
@141345 141345 reopened this Aug 25, 2023
@hrishibhat
Copy link

This issue is not included in the contest pool rewards

@arjun-io
Copy link
Author

Fixed: equilibria-xyz/perennial-v2#58

@MLON33
Copy link

MLON33 commented Sep 14, 2023

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.

@sherlock-audit sherlock-audit deleted a comment from jacksanford1 Sep 14, 2023
@arjun-io
Copy link
Author

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: equilibria-xyz/perennial-v2#100

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Medium A valid Medium severity issue Non-Reward This issue will not receive a payout Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

4 participants