tsvetanovv - It is possible to revert because overflow in `getPhaseSwitchoverData`

[sherlock-admin]

tsvetanovv

medium

It is possible to revert because overflow in getPhaseSwitchoverData

Summary

In ChainlinkAggregator.sol we have getPhaseSwitchoverData() function. The function returns the round count and the next phase starting round for the lastSyncedRound phase. But is possible to overflow under certain conditions.

Vulnerability Detail

Let's look at the following code from the function:

86:        ChainlinkRound memory lastSyncedRound = getRound(self, lastSyncedRoundId);
87:        uint16 phaseToSearch = lastSyncedRound.phaseId();
88:        while (nextPhaseStartingRoundId == 0) {
89:            phaseToSearch++;
90:            nextPhaseStartingRoundId = getStartingRoundId(self, phaseToSearch, lastSyncedRound.timestamp);
91:        }

This portion of the code determines where the next phase starts if the lastSyncedRound is the final round in its phase before latestRound.

The problem here is that the phaseToSearch variable can be very close to the maximum number of uint16 because phaseId return uint16.

87:        uint16 phaseToSearch = lastSyncedRound.phaseId();

27:    function phaseId(ChainlinkRound memory self) internal pure returns (uint16) {
28:        return uint16(self.roundId >> PHASE_OFFSET);
29:    }

So we run into a situation where phaseToSearch can be very close to the maximal number of uint16 and the function can immediately revert because of overflow in the while loop.

89:      phaseToSearch++;

Impact

getPhaseSwitchoverData() function revert because overflow

Code Snippet

https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial-oracle/contracts/types/ChainlinkAggregator.sol#L61-L94

Tool used

Manual Review

Recommendation

Change uint16 phaseToSearch to a larger number. You can make it uint32 or uint64