Steadefi
Steadefi
DeFiHardhatFoundryOracle
35,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

ChainlinkARBOracle.sol#L249-L268 : Incorrect check leads to misconfiguration of `max delay` and `max deviation`

ak1

Summary

ChainlinkARBOracle contract is used to fetch the token price. The price is validated whether it is in allowable limit.
In order to check if the price is in the allowable limit, max token delay and max deviation values are used to compare the last round price value with current round.

The functions addTokenMaxDelay and addTokenMaxDeviation are used to set these max deviation and max delay values by the admin.

the condition checks used to validate these two parameters are having flaw which is allowing to configure to zero value.

Vulnerability Details

https://github.com/Cyfrin/2023-10-SteadeFi/blob/0f909e2f0917cb9ad02986f631d622376510abec/contracts/oracles/ChainlinkARBOracle.sol#L249-L268

function addTokenMaxDelay(address token, uint256 maxDelay) external onlyOwner {
if (token == address(0)) revert Errors.ZeroAddressNotAllowed();
if (feeds[token] == address(0)) revert Errors.NoTokenPriceFeedAvailable();
if (maxDelay < 0) revert Errors.TokenPriceFeedMaxDelayMustBeGreaterOrEqualToZero(); ------->> audit find
​
​
maxDelays[token] = maxDelay;
}
​
​
/**
* @notice Add Chainlink max deviation for token
* @param token Token address
* @param maxDeviation Max deviation allowed in seconds
*/
function addTokenMaxDeviation(address token, uint256 maxDeviation) external onlyOwner {
if (token == address(0)) revert Errors.ZeroAddressNotAllowed();
if (feeds[token] == address(0)) revert Errors.NoTokenPriceFeedAvailable();
if (maxDeviation < 0) revert Errors.TokenPriceFeedMaxDeviationMustBeGreaterOrEqualToZero(); ---------->> audit find
​
​
maxDeviations[token] = maxDeviation;
}

Both the uint256 maxDelay and uint256 maxDeviation are unsigned int values which never accept value less than zero.

If the values are passed as zero, the functions are allowing to configure these values incorrectly.

Impact

This leads to setting the max delay and max time as zero value.
This breaks one of the important invariant check inside the function _badPriceDeviation

Tools Used

Manual review.

Recommendations

we suggest to update the following functions as shown below.

function addTokenMaxDelay(address token, uint256 maxDelay) external onlyOwner {
if (token == address(0)) revert Errors.ZeroAddressNotAllowed();
if (feeds[token] == address(0)) revert Errors.NoTokenPriceFeedAvailable();
if (maxDelay < 0) revert Errors.TokenPriceFeedMaxDelayMustBeGreaterOrEqualToZero(); ----
​
if (maxDelay > 0) revert Errors.TokenPriceFeedMaxDelayMustBeGreaterOrEqualToZero(); ++++
​
​
maxDelays[token] = maxDelay;
}
​
​
/**
* @notice Add Chainlink max deviation for token
* @param token Token address
* @param maxDeviation Max deviation allowed in seconds
*/
function addTokenMaxDeviation(address token, uint256 maxDeviation) external onlyOwner {
if (token == address(0)) revert Errors.ZeroAddressNotAllowed();
if (feeds[token] == address(0)) revert Errors.NoTokenPriceFeedAvailable();
if (maxDeviation < 0) revert Errors.TokenPriceFeedMaxDeviationMustBeGreaterOrEqualToZero(); -----
if (maxDeviation > 0) revert Errors.TokenPriceFeedMaxDeviationMustBeGreaterOrEqualToZero();++++++
​
​
maxDeviations[token] = maxDeviation;
}
Updates

Lead Judging Commences

hans Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

INFO: Unnecessary maxDelay/maxDeviation check

Redundant check on maxDelay and/or maxDeviation in ARBOracle

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.