XO-_search function does not check for minRoundId being equal to 0
Summary
The _search functio has a vulnerability that can lead to incorrect results. The problem arises from the initialization of the maxRoundId variable, which is set as minRoundId + 1000 without checking if minRoundId is equal to 0. This can be exploited by an attacker to obtain incorrect price information for tokens.
Vulnerability Detail
function _search(AggregatorProxyInterface proxy, uint16 phaseId, uint256 targetTimestamp, uint256 minTimestamp, uint256 minRoundId) private view returns (uint256) {
uint256 maxRoundId = minRoundId + 1000; // Start 1000 rounds away when searching for maximum
uint256 maxTimestamp = _tryGetProxyRoundData(proxy, phaseId, uint80(maxRoundId));
// Find the round bounds of the phase to perform the binary search
while (maxTimestamp <= targetTimestamp) {
minRoundId = maxRoundId;
minTimestamp = maxTimestamp;
maxRoundId = maxRoundId * 2; // Find bounds of phase by multiplying the max round by 2
maxTimestamp = _tryGetProxyRoundData(proxy, phaseId, uint80(maxRoundId));
}
// Binary Search starts here. The algorithm calculates the middle round ID and finds it's timestamp
// If the midtimestamp is greater than target, set max to mid and continue
// If the midtimestamp is less than or equal to target, set min to mid and continue
// Exit when min + 1 is equal to or greater than max (no rounds between them)
while (minRoundId + 1 < maxRoundId) {
uint256 midRound = Math.average(minRoundId, maxRoundId);
uint256 midTimestamp = _tryGetProxyRoundData(proxy, phaseId, uint80(midRound));
if (midTimestamp > targetTimestamp) {
maxTimestamp = midTimestamp;
maxRoundId = midRound;
} else {
minTimestamp = midTimestamp;
minRoundId = midRound;
}
}
// If the found timestamp is not greater than target timestamp or no max was found, then the desired round does
// not exist in this phase
if (maxTimestamp <= targetTimestamp || maxTimestamp == type(uint256).max) return 0;
return _aggregatorRoundIdToProxyRoundId(phaseId, uint80(maxRoundId));
}
There is a vulnerability in the _search function. This function is used to find the round ID closest to but greater than targetTimestamp for the specified phase ID, the problem in this function is in the the maxRoundId variable is initialized to minRoundId + 1000, here there is no check to ensure that minRoundId is not equal to 0.
This means that if minRoundId is equal to 0, then maxRoundId will be equal to 1000, which is outside of the bounds of the phase. This could lead to an attacker being able to exploit the bug to get an incorrect result, for example, if the attacker wants to get an incorrect price for a token, they could set targetTimestamp to a value that is before the start of the phase. This would cause the _search function to return a round ID that is outside of the bounds of the phase. The attacker could then use this round ID to get an incorrect price for the token.
Impact
An attacker can take advantage of this vulnerability by manipulating the targetTimestamp to a value before the start of the phase. When the _search function is called, it will return a round ID that is beyond the actual bounds of the phase. By using this incorrect round ID, the attacker can obtain misleading price information for tokens.
adds a check to ensure that minRoundId is not equal to 0 before calculating maxRoundId this will ensures that maxRoundId is always within the bounds of the phase.
XDZIBEC
high
XO-
_search
function does not check forminRoundId
being equal to0
Summary
The
_search
functio has a vulnerability that can lead to incorrect results. The problem arises from the initialization of themaxRoundId
variable, which is set asminRoundId + 1000
without checking ifminRoundId
is equal to0
. This can be exploited by an attacker to obtain incorrect price information for tokens.Vulnerability Detail
_search
function. This function is used to find the roundID
closest to but greater thantargetTimestamp
for the specified phaseID
, the problem in this function is in the themaxRoundId
variable isinitialized
tominRoundId + 1000
, here there is no check to ensure thatminRoundId
is not equal to0
.minRoundId
is equal to0
, thenmaxRoundId
will be equal to1000
, which is outside of the bounds of the phase. This could lead to an attacker being able to exploit the bug to get an incorrect result, for example, if the attacker wants to get an incorrect price for a token, they could settargetTimestamp
to a value that is before the start of the phase. This would cause the_search
function to return a roundID
that is outside of the bounds of the phase. The attacker could then use this roundID
to get an incorrect price for thetoken
.Impact
targetTimestamp
to a value before the start of the phase. When the_search
function is called, it will return a roundID
that is beyond the actual bounds of the phase. By using this incorrect roundID
, the attacker can obtain misleading price information for tokens.Code Snippet
Tool used
Manual Review
Recommendation
minRoundId
is not equal to0
before calculatingmaxRoundId
this will ensures thatmaxRoundId
is always within the bounds of the phase.