computeExchangeRate() can be replaced by requiring >= INITIAL_RESERVES amount of deposit in doActivateMarket()Per the doc:
computeExchangeRate has been modified to support the case where ETokens exist on the pool but no actual deposits have been made yet.
function computeExchangeRate(AssetCache memory assetCache) private pure returns (uint) {
uint totalAssets = assetCache.poolSize + (assetCache.totalBorrows / INTERNAL_DEBT_PRECISION);
if (totalAssets == 0 || assetCache.totalBalances == 0) return 1e18;
return totalAssets * 1e18 / assetCache.totalBalances;
}
We believe this will create a special and inconsistent state where totalAssets == 0, exchangeRate == 1e18 and totalBalances > 0, in which case, totalAssets != totalBalances * exchangeRate.
Another approach to solving the problem will be to change the Markets#doActivateMarket(), and require the caller to deposit at least INITIAL_RESERVES of the underlying token at an exchangeRate of 1e18.
For example:
If the underlying asset is DAI with a decimals of 18, then the caller must deposit 1e6 wei (INITIAL_RESERVES == 1e6) of DAI, the initial reserveBalance and totalBalances will be 1e6; totalAssets and assetCache.poolSize will also be 1e6.
If the underlying asset is WBTC with a decimals of 8, then the caller must deposit 1 wei of WBTC, the initial reserveBalance and totalBalances will be 1e10; totalAssets and assetCache.poolSize will also be 1e10.
DToken.sol#flashLoan() can be used for exchangeRate manipulationThe newly added changes introduced in PR #148: make view methods that loadAssetCacheRO check the re-entrancy guard is unlocked have rendered this issue invalid.
function flashLoan(uint amount, bytes calldata data) external nonReentrant {
(address underlying,,, address msgSender) = CALLER();
uint origBalance = IERC20(underlying).balanceOf(address(this));
Utils.safeTransfer(underlying, msgSender, amount);
IFlashLoan(msgSender).onFlashLoan(data);
require(IERC20(underlying).balanceOf(address(this)) >= origBalance, "e/flash-loan-not-repaid");
}
A set of public/external functions will utilize computeExchangeRate() to provide information about underlying value to the caller:
For instances:
balanceOfUnderlying(): https://github.com/euler-xyz/euler-contracts/blob/0fade57d9ede7b010f943fa8ad3ad74b9c30e283/contracts/modules/EToken.sol#L79-L84convertBalanceToUnderlying() https://github.com/euler-xyz/euler-contracts/blob/0fade57d9ede7b010f943fa8ad3ad74b9c30e283/contracts/modules/EToken.sol#L104-L112convertUnderlyingToBalance() https://github.com/euler-xyz/euler-contracts/blob/0fade57d9ede7b010f943fa8ad3ad74b9c30e283/contracts/modules/EToken.sol#L114-L122However, computeExchangeRate() will read the real time balanceOf as poolSize and compute the exchange rate based on that:
DToken.sol#flashLoan() makes it possible to manipulate the exchangeRate (downwards) very easily and at almost 0 cost, simply by taking a huge amount of underlying out of the contract and use the manipulated price in IFlashLoan(msgSender).onFlashLoan(data)/
This can cause problems if the exchange rate based public/external functions listed above are being used in 3rd party protocols for important accounting related computations.
RiskManager#callChainlinkLatestAnswer() may cause a stale price to be usedfunction callChainlinkLatestAnswer(address chainlinkAggregator) private view returns (uint price) {
(bool success, bytes memory data) = chainlinkAggregator.staticcall(abi.encodeWithSelector(IChainlinkAggregatorV2V3.latestAnswer.selector));
if (!success) {
return 0;
}
int256 answer = abi.decode(data, (int256));
if (answer <= 0) {
return 0;
}
price = uint(answer);
if (price > 1e36) price = 1e36;
}
Chainlink's latestAnswer() wont return the updated time, therefore, the freshness check can not be done. This could lead to stale prices being used.
If the market price of the token drops very quickly ("flash crashes"), and Chainlink's feed does not get updated in time, the Euler protocol will continue to believe the token is worth more than the market value.
Attackers can exploit it by depositing huge amounts of overvalued tokens as collateral and borrowing good assets, as the market value of the collateral is actually lower than the borrowed funds, those borrowers won't repay their debts.
A similar issue caused the Venus protocol on BSC to accrue $14.2M of bad debt during the lunacrash.
Consider using latestRoundData() instead and adding missing checks for stale price:
(uint80 roundID, int256 answer, , uint256 updatedAt, uint80 answeredInRound) = chainlinkAggregator.latestRoundData();
if (block.timestamp - updatedAt > validPeriod)) {
return 0;
}
if (answer <= 0) {
return 0;
}
...
newPricingType in setPricingConfig()function setPricingConfig(address underlying, uint16 newPricingType, uint32 newPricingParameter) external nonReentrant governorOnly {
address eTokenAddr = underlyingLookup[underlying].eTokenAddress;
require(eTokenAddr != address(0), "e/gov/underlying-not-activated");
require(newPricingType < PRICINGTYPE__OUT_OF_BOUNDS, "e/gov/bad-pricing-type");
AssetStorage storage assetStorage = eTokenLookup[eTokenAddr];
AssetCache memory assetCache = loadAssetCache(underlying, assetStorage);
assetStorage.pricingType = assetCache.pricingType = newPricingType;
assetStorage.pricingParameters = assetCache.pricingParameters = newPricingParameter;
if (newPricingType == PRICINGTYPE__CHAINLINK) {
require(chainlinkPriceFeedLookup[underlying] != address(0), "e/gov/chainlink-price-feed-not-initialized");
}
emit GovSetPricingConfig(underlying, newPricingType, newPricingParameter);
}
While it checks for newPricingType not exceeding the upper bound, there is no check to ensure it's greater than the lower bound of 0.
Change to:
function setPricingConfig(address underlying, uint16 newPricingType, uint32 newPricingParameter) external nonReentrant governorOnly {
address eTokenAddr = underlyingLookup[underlying].eTokenAddress;
require(eTokenAddr != address(0), "e/gov/underlying-not-activated");
require(newPricingType > 0 && newPricingType < PRICINGTYPE__OUT_OF_BOUNDS, "e/gov/bad-pricing-type");
loadAssetCache() and memory writes to assetCache is unnecessary in setPricingConfig()function setPricingConfig(address underlying, uint16 newPricingType, uint32 newPricingParameter) external nonReentrant governorOnly {
address eTokenAddr = underlyingLookup[underlying].eTokenAddress;
require(eTokenAddr != address(0), "e/gov/underlying-not-activated");
require(newPricingType < PRICINGTYPE__OUT_OF_BOUNDS, "e/gov/bad-pricing-type");
AssetStorage storage assetStorage = eTokenLookup[eTokenAddr];
AssetCache memory assetCache = loadAssetCache(underlying, assetStorage);
assetStorage.pricingType = assetCache.pricingType = newPricingType;
assetStorage.pricingParameters = assetCache.pricingParameters = newPricingParameter;
if (newPricingType == PRICINGTYPE__CHAINLINK) {
require(chainlinkPriceFeedLookup[underlying] != address(0), "e/gov/chainlink-price-feed-not-initialized");
}
emit GovSetPricingConfig(underlying, newPricingType, newPricingParameter);
}
assetCache at L56-57 are unnecessary as assetCache wont be read again;loadAssetCache() is unnecessary as assetCache will not be read, and changing pricing config won't impact interests. Therefore, the settlement of interests in loadAssetCache() -> initAssetCache() can be done later or we could say it's not relevant.Consdier removing loadAssetCache() and the memory writes to assetCache at L56-57.
Governance.sol#convertReserves() Validation of reserveBalance can be done earlier to save gasfunction convertReserves(address underlying, address recipient, uint amount) external nonReentrant governorOnly {
address eTokenAddress = underlyingLookup[underlying].eTokenAddress;
require(eTokenAddress != address(0), "e/gov/underlying-not-activated");
updateAverageLiquidity(recipient);
AssetStorage storage assetStorage = eTokenLookup[eTokenAddress];
AssetCache memory assetCache = loadAssetCache(underlying, assetStorage);
if (amount == type(uint).max) amount = assetStorage.reserveBalance - INITIAL_RESERVES;
require(amount <= assetStorage.reserveBalance, "e/gov/insufficient-reserves");
assetStorage.reserveBalance = assetCache.reserveBalance = assetCache.reserveBalance - uint96(amount);
// Decrease totalBalances because increaseBalance will increase it by amount
assetStorage.totalBalances = assetCache.totalBalances = encodeAmount(assetCache.totalBalances - amount);
require(assetStorage.reserveBalance >= INITIAL_RESERVES, "e/gov/reserves-depleted");
increaseBalance(assetStorage, assetCache, eTokenAddress, recipient, amount);
if (assetStorage.users[recipient].owed != 0) checkLiquidity(recipient);
logAssetStatus(assetCache);
emit GovConvertReserves(underlying, recipient, balanceToUnderlyingAmount(assetCache, amount));
}
Change to:
function convertReserves(address underlying, address recipient, uint amount) external nonReentrant governorOnly {
address eTokenAddress = underlyingLookup[underlying].eTokenAddress;
require(eTokenAddress != address(0), "e/gov/underlying-not-activated");
updateAverageLiquidity(recipient);
AssetStorage storage assetStorage = eTokenLookup[eTokenAddress];
require(assetStorage.reserveBalance >= INITIAL_RESERVES, "e/gov/reserves-depleted");
AssetCache memory assetCache = loadAssetCache(underlying, assetStorage);
uint maxAmount = assetCache.reserveBalance - INITIAL_RESERVES;
if (amount == type(uint).max) amount = maxAmount;
require(amount <= maxAmount, "e/gov/insufficient-reserves");
assetStorage.reserveBalance = assetCache.reserveBalance = assetCache.reserveBalance - uint96(amount);
// Decrease totalBalances because increaseBalance will increase it by amount
assetStorage.totalBalances = assetCache.totalBalances = encodeAmount(assetCache.totalBalances - amount);
increaseBalance(assetStorage, assetCache, eTokenAddress, recipient, amount);
if (assetStorage.users[recipient].owed != 0) checkLiquidity(recipient);
logAssetStatus(assetCache);
emit GovConvertReserves(underlying, recipient, balanceToUnderlyingAmount(assetCache, amount));
}
The eIP introduced a new setChainlinkPriceFeed function allows the configuration of a chainlink feed for an asset.
Per to Chainlink's docs:
Chainlink Price Feeds do not provide streaming data. Rather, the aggregator updates its latestAnswer when the value deviates beyond a specified threshold or when the heartbeat idle time has passed. You can find the heartbeat and deviation values for each data feed at data.chain.link or in the Contract Addresses lists.
And the Deviation and Heartbeat on Ethereum Mainnet can be quite large and long, usually 2% for Deviation and 24h as Heartbeat.
Source: https://docs.chain.link/docs/ethereum-addresses/
This means that the price of Chainlink feed can be significantly delayed and deviated in comparison to the real time Uniswap v3 TWAP price.
As a result, the liquidation of assets priced with a chainlink feed, may not be as timely as the assets priced based on Uniswap v3's TWAP.
There are no immediate risk from using chainlink feed, but we encourage you to check and confirm the Deviation and Heartbeat for the feed to be added, and also consider setting a lower collateral factor ratio to mitigate the risk of delayed liquidation due to delayed oracle price.