Summary
The LibOracle.sol:getOraclePrice() function can return an incorrect price for assets that have a Chainlink oracle with a different number of decimals than the Chainlink oracle for the ETH/USD pair.
Vulnerability Details
LibOracle.sol:getOraclePrice() is used to get the price of an asset in ETH.
First, the price for the ETH/USD pair is fetched from the Chainlink oracle. If asset is other than USD a second call is made to the Chainlink oracle to get the price of asset/USD pair. The price of asset/ETH is then calculated by dividing the price of asset/USD by the price of ETH/USD.
However, it is not taken into account that the number of decimals of the asset/USD pair may be different from the number of decimals of the ETH/USD pair. This can lead to an incorrect price of asset/ETH being returned.
Note also that the resultant price could be greater than type(uint80).max, which would cause an overflow in the setPriceAndTime() function.
132 function setPriceAndTime(address asset, uint256 oraclePrice, uint32 oracleTime)
133 internal
134 {
135 AppStorage storage s = appStorage();
136 s.bids[asset][Constants.HEAD].ercAmount = uint80(oraclePrice); 👈
137 s.bids[asset][Constants.HEAD].creationTime = oracleTime;
138 }
Proof of Concept
Add a new file with the following code into test/fork and run forge test -vv --mt testFork_getOraclePrice.
pragma solidity 0.8.21;
import {ConstantsTest} from "test/utils/ConstantsTest.sol";
import {U256} from "contracts/libraries/PRBMathHelper.sol";
import {Modifiers} from "contracts/libraries/AppStorage.sol";
import {LibOracle} from "contracts/libraries/LibOracle.sol";
import {AggregatorV3Interface} from
"@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";
import "forge-std/console2.sol";
contract MockDiamond is Modifiers {
constructor(address baseAsset, address baseOracle, address asset, address assetOracle) {
s.baseOracle = baseOracle;
s.asset[baseAsset].oracle = baseOracle;
s.asset[asset].oracle = assetOracle;
}
function getOraclePrice(address asset) public view returns (uint256) {
return LibOracle.getOraclePrice(asset);
}
}
contract ChainlinkForkTest is ConstantsTest {
using U256 for uint256;
uint256 public mainnetFork;
address public baseAsset = address(0x111);
address public baseOracle = 0x5f4eC3Df9cbd43714FE2740f5E3616155c5b8419;
address public asset = address(0x222);
address public assetOracle = 0xe20CA8D7546932360e37E9D72c1a47334af57706;
MockDiamond diamond;
uint256 public forkBlock = 18_111_111;
function setUp() public {
try vm.envString("MAINNET_RPC_URL") returns (string memory rpcUrl) {
mainnetFork = vm.createSelectFork(rpcUrl, forkBlock);
} catch {
revert("env: MAINNET_RPC_URL failure");
}
assertEq(vm.activeFork(), mainnetFork);
diamond = new MockDiamond(baseAsset, baseOracle, asset, assetOracle);
}
function testFork_getOraclePrice() public {
uint256 assetPrice = diamond.getOraclePrice(asset);
console2.log("assetPrice: %s", assetPrice);
}
}
The console output is:
Logs:
assetPrice: 6748028520519842969307637
This translates to a price of 6,748,028.52 ETH per AMPL, while the value of the AMPL is just above 1 USD.
Impact
Assets with a Chainlink oracle that has a different number of decimals than the Chainlink oracle for the ETH/USD pair will have an incorrect price returned by LibOracle.sol:getOraclePrice(), which can lead to many problems, including users being required to deposit more/less collateral than expected or being liquidated when they shouldn't be.
Tools Used
Manual inspection.
Recommendations
File: contracts/libraries/LibOracle.sol
) = oracle.latestRoundData();
- uint256 priceInEth = uint256(price).div(uint256(basePrice));
+ uint8 BASE_DECIMALS = 8;
+ uint8 oracleDecimals = oracle.decimals();
+ uint256 normalizedPrice;
+ if (oracleDecimals > BASE_DECIMALS) {
+ normalizedPrice = uint256(price) / (10 ** (oracleDecimals - BASE_DECIMALS));
+ } else {
+ normalizedPrice = uint256(price) * (10 ** (8 - oracleDecimals));
+ }
+ uint256 priceInEth = normalizedPrice.div(uint256(basePrice));
It would also be a good idea to add a check to ensure that the price is not greater than type(uint80).max.
