DittoETH
Ditto
DeFiFoundryOracle
55,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

Divide by 0 revert in LibOracle::getOraclePrice

nican0r

Summary

In the getOraclePrice function it fetches the basePrice from the ETH/USD Chainlink oracle and uses this to calculate prices for DittoAssets other than cUSD (when oracle != baseOracle). However there's no check in this branch to handle the chainlink oracle returning basePrice = 0 as there is in the oracle == baseOracle so instead of triggering the oracleCircuitBreaker function it reverts with a divide by 0 error. If the frontend is expecting the InvalidPrice error from the oracleCircuitBreaker in order to trigger the fallback to the Uniswap TWAP price oracle it would not get triggered and therefore the price in the system would remain the same after the update.

Vulnerability Details

The oracleCircuitBreaker function called by getOraclePrice has a condition where it checks baseChainlinkPrice <= 0

function oracleCircuitBreaker(
...
int256 baseChainlinkPrice,
...
) private view {
bool invalidFetchData =
...
baseChainlinkPrice <= 0;
​
if (invalidFetchData) revert Errors.InvalidPrice();
}

therefore it expects the baseChainlinkPrice passed in to be 0 in certain cases that would trigger falling back to the Uniswap oracle. But due to the calculation of priceInEth as:

uint256 priceInEth = uint256(price).div(uint256(basePrice))
oracleCircuitBreaker(
...
basePrice,
...
);

the function would revert early with a divide by 0 error if basePrice = 0 instead of throwing the expected InvalidPrice error from oracleCircuitBreaker. There is a check in the oracle == baseOracle branch that handles this by setting basePriceInEth = 0 if the basePrice returned by the oracle <= 0 :

uint256 basePriceInEth = basePrice > 0
? uint256(basePrice * Constants.BASE_ORACLE_DECIMALS).inv()
: 0;

but because this check isn't made in the specified branch it reverts early as described above.

If the frontend is expecting to catch this InvalidPrice error in order to fall back to the Uniswap oracle, it would not end up falling back and the price of the asset would remain un-updated. This would cause subsequent calls to create an order on the OB to revert, DOSing anyone from creating new orders until the chainlink oracle comes back online if the last update time of the oracle is > 15 minutes because it would cause the call to getOraclePrice to revert in getSavedOrSpotOraclePrice when it's called in the order creation flow:

if (LibOrders.getOffsetTime() - getTime(asset) < 15 minutes) {
return getPrice(asset);
} else {
return getOraclePrice(asset);
}

This is different from the "non ETH/USD oracle assets currently have no Uniswap TWAP fallback, it reverts" known issue because it is not dependent on the non ETH/USD oracle returning 0, it only requires the ETH/USD oracle returning 0 which the oracleCircuitBreaker is supposed to be able to handle but doesn't.

Impact

User is denied the ability to create new orders when chainlink ETH/USD oracle is down so they can't enter/exit the system because there's no fallback to the Uniswap TWAP oracle.

Tools Used

Manual Review

Recommendations

Provide the same check for basePrice <= 0 in the oracle == baseOracle case to the oracle != baseOracle case:

uint256 basePriceInEth = basePrice > 0
? uint256(basePrice * Constants.BASE_ORACLE_DECIMALS).inv()
: 0;
uint256 priceInEth = uint256(price).div(uint256(basePrice));
Updates

Lead Judging Commences

0xnevi Lead Judge about 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-188

Support

FAQs

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