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

0x52 - Funding settlement will be DOS'd for a time after the phaseID change of an underlying chainlink aggregator #177

Open
sherlock-admin opened this issue Jul 3, 2023 · 1 comment
Labels
Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Won't Fix The sponsor confirmed this issue will not be fixed

Comments

@sherlock-admin
Copy link
Contributor

0x52

medium

Funding settlement will be DOS'd for a time after the phaseID change of an underlying chainlink aggregator

Summary

Oracle incorrectly assumes that roundID is always incremented by one but this is not the case. Chainlink's roundID is actually two values packed together: phaseId and aggregatorRoundId. When the phaseID is incremented the roundID increases by 2 ** 64. After a phaseID increment all calls to settle funding will revert until an entire funding interval has elapsed. Since all markets are settled simultaneously, even a single oracle incrementing will result in all market funding being DOS'd. Although this funding can be made up later it will user different TWAP values which will result in users being paid differently causing loss of yield to a portion of all users.

Vulnerability Detail

https://snowtrace.io/address/0x976b3d034e162d8bd72d6b9c989d545b839003b0#code#L206

  function getAnswer(uint256 _roundId)
    public
    view
    virtual
    override
    returns (int256 answer)
  {
    if (_roundId > MAX_ID) return 0;

    (uint16 phaseId, uint64 aggregatorRoundId) = parseIds(_roundId);
    AggregatorV2V3Interface aggregator = phaseAggregators[phaseId];
    if (address(aggregator) == address(0)) return 0;

    return aggregator.getAnswer(aggregatorRoundId);
  }

The above code is from the ETH/USD aggregator on AVAX, It can be seen that the roundId is made up of 2 packed components, the phaseId and aggregatorRoundId. As explained in the summary, when the phaseId is incremented 2 ** 64 "rounds" will be skipped. When currentRound - 1 is inevitably queried after this increment, the call will revert because that round doesn't exist this DOS will last for up to 24 hours depending on market settings. After the DOS ends, settingFunding will be able to catch up but it will now calculate the funding rate with different TWAP values.

Impact

Loss of yield to a portion of all users in every market each time there is a phaseId shift

Code Snippet

Tool used

Manual Review

Recommendation

I would recommend using a try block when calling the aggregator. If the roundID is nonzero and is reverting then the oracle needs try again with a lower phaseId

@github-actions github-actions bot added the Medium A valid Medium severity issue label Jul 10, 2023
@0xshinobii 0xshinobii added the Sponsor Confirmed The sponsor acknowledged this issue is valid label Jul 18, 2023
@0xshinobii
Copy link

This is a valid issue. We will fix it with post-mainnet releases because we are using trusted oracle (deployed on hubbleNet) with mainnet release and any updates to that will be notified prior.

@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Jul 19, 2023
@0xshinobii 0xshinobii added the Won't Fix The sponsor confirmed this issue will not be fixed label Aug 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Won't Fix The sponsor confirmed this issue will not be fixed
Projects
None yet
Development

No branches or pull requests

2 participants