On April 7th, after Aave V3 had launched for 3 weeks, we discovered an issue on Aave V3’s price oracle. To be more specifically, the fallback oracle. After initially failing to reach out to Aave, we contacted @samczsun who helped notify the Aave team and the issue was resolved promptly. Although not easy to exploit, if exploited, would allow anyone to drain funds on Aave V3 across all L2 deployments. (worth >$2.9B at the time of writing).
Five different security firms have audited Aave V3, including world-renowned names like OpenZeppelin and Trail of Bits. Yet, because Aave deployed unreviewed contracts designed for testing. We will describe the issue, how it wasn’t caught in security audits, and how to avoid similar bugs in the future.
What is a Price Oracle
In order for lending protocols to determine the price of an asset, a price oracle is used to fetch prices either on-chain or off-chain. On-chain oracles have suffered a lot of problems that allow price manipulation. Therefore Aave relies on an off-chain oracle, Chainlink, for price reporting. This is more secure because prices are fetched from various sources (e.g. exchanges) from trusted parties.
The vulnerability
While Chainlink oracle is secure and battle-tested, protocols still need to correctly integrate it into their contracts.
The following is the code of interest in AaveOracle.sol.
function getAssetPrice(address asset) public view override returns (uint256) {
AggregatorInterface source = assetsSources[asset]; if (asset == BASE_CURRENCY) {
return BASE_CURRENCY_UNIT;
} else if (address(source) == address(0)) {
return _fallbackOracle.getAssetPrice(asset);
} else {
int256 price = source.latestAnswer();
if (price > 0) {
return uint256(price);
} else {
return _fallbackOracle.getAssetPrice(asset);
}
}
}
In normal cases, the price is fetched from Chainlink via AggregatorInterface.latestAnswer. There are two scenario where a fallback oracle is used:
- The asset’s price feed is not set:
if (address(source) == address(0)) - Chainlink reports price = 0: inverse of
if (price > 0)
Now, let’s take a look at what the fallback oracle looks like, using Aave V3 on Polygon as an example.
https://polygonscan.com/tx/0xecff0cf908300c0b205b426a2d0d858e0769f6f70db312698365f2569f97e86e
From the transaction that calls setFallbackoracle we know the fallback oracle contract is set to https://polygonscan.com/address/0xaA5890362f36FeaAe91aF248e84e287cE6eCD1A9#code. The source code is verified and we can have a glimpse on how PriceOracle is written.
// SPDX-License-Identifier: BUSL-1.1
pragma solidity 0.8.10;import {IPriceOracle} from '../../interfaces/IPriceOracle.sol';contract PriceOracle is IPriceOracle {
// Map of asset prices (asset => price)
mapping(address => uint256) internal prices; uint256 internal ethPriceUsd; event AssetPriceUpdated(address asset, uint256 price, uint256 timestamp);
event EthPriceUpdated(uint256 price, uint256 timestamp); function getAssetPrice(address asset) external view override returns (uint256) {
return prices[asset];
} function setAssetPrice(address asset, uint256 price) external override {
prices[asset] = price;
emit AssetPriceUpdated(asset, price, block.timestamp);
} function getEthUsdPrice() external view returns (uint256) {
return ethPriceUsd;
} function setEthUsdPrice(uint256 price) external {
ethPriceUsd = price;
emit EthPriceUpdated(price, block.timestamp);
}
}
It shouldn’t take you 1 minute to realize that the function setAssetPrice does not have any access controls. In other words, anyone can set any price for any asset.
Impact and Exploitability
A malicious actor could manipulate the oracle, have it report an astronomical number for an asset’s price, deposit a very small amount of the asset, then proceed to borrow all available funds on Aave.
We came up with 2 scenario where this would become exploitable.
Chainlink reporting price = 0
Theoretically, it is almost impossible for an asset to be valued at $0. However, because Aave uses the deprecated function latestAnswer instead of latestRoundData as pointed out by Trail of Bits, if no answer is reached, the said function could return 0.
Aave adding an asset with an incorrect order
If a new asset was added as a collateral before the price feed of the asset is configured, there would be a brief moment where the fallback oracle was used, and hence manipulated.
A incorrect order would look like this.
1. Enable aToken
2. Set price feed for aToken’s underlying asset
This wouldn’t be a problem if a faulty fallback oracle was not used, because the price would then just be 0 and so be the borrowing power. We looked at Aave’s documentations and couldn’t find if there’s a standard procedure for adding a new asset.
Is Aave V2 vulnerable?
The codebase for Aave V3 is very similar to Aave V2. In fact, their oracle contracts are almost identical.
However, Aave v2 uses a fallback oracle that has proper access controls.
https://etherscan.io/address/0x5b09e578cfeaa23f1b11127a658855434e4f3e09#code
contract AaveFallbackOracle is Ownable, IPriceOracleGetter {
using SafeMath for uint256; struct Price {
uint64 blockNumber;
uint64 blockTimestamp;
uint128 price;
} event PricesSubmitted(address sybil, address[] assets, uint128[] prices);
event SybilAuthorized(address indexed sybil);
event SybilUnauthorized(address indexed sybil); uint256 public constant PERCENTAGE_BASE = 1e4; mapping(address => Price) private _prices; mapping(address => bool) private _sybils; modifier onlySybil {
_requireWhitelistedSybil(msg.sender);
_;
} function authorizeSybil(address sybil) external onlyOwner {
_sybils[sybil] = true; emit SybilAuthorized(sybil);
} function unauthorizeSybil(address sybil) external onlyOwner {
_sybils[sybil] = false; emit SybilUnauthorized(sybil);
} function submitPrices(address[] calldata assets, uint128[] calldata prices) external onlySybil {
require(assets.length == prices.length, 'INCONSISTENT_PARAMS_LENGTH');
for (uint256 i = 0; i < assets.length; i++) {
_prices[assets[i]] = Price(uint64(block.number), uint64(block.timestamp), prices[i]);
} emit PricesSubmitted(msg.sender, assets, prices);
} function getAssetPrice(address asset) external view override returns (uint256) {
return uint256(_prices[asset].price);
} function isSybilWhitelisted(address sybil) public view returns (bool) {
return _sybils[sybil];
} function getPricesData(address[] calldata assets) external view returns (Price[] memory) {
Price[] memory result = new Price[](assets.length);
for (uint256 i = 0; i < assets.length; i++) {
result[i] = _prices[assets[i]];
}
return result;
} function filterCandidatePricesByDeviation(
uint256 deviation,
address[] calldata assets,
uint256[] calldata candidatePrices
) external view returns (address[] memory, uint256[] memory) {
require(assets.length == candidatePrices.length, 'INCONSISTENT_PARAMS_LENGTH');
address[] memory filteredAssetsWith0s = new address[](assets.length);
uint256[] memory filteredCandidatesWith0s = new uint256[](assets.length);
uint256 end0sInLists;
for (uint256 i = 0; i < assets.length; i++) {
uint128 currentOraclePrice = _prices[assets[i]].price;
if (
uint256(currentOraclePrice) >
candidatePrices[i].mul(PERCENTAGE_BASE.add(deviation)).div(PERCENTAGE_BASE) ||
uint256(currentOraclePrice) <
candidatePrices[i].mul(PERCENTAGE_BASE.sub(deviation)).div(PERCENTAGE_BASE)
) {
filteredAssetsWith0s[end0sInLists] = assets[i];
filteredCandidatesWith0s[end0sInLists] = candidatePrices[i];
end0sInLists++;
}
}
address[] memory resultAssets = new address[](end0sInLists);
uint256[] memory resultPrices = new uint256[](end0sInLists);
for (uint256 i = 0; i < end0sInLists; i++) {
resultAssets[i] = filteredAssetsWith0s[i];
resultPrices[i] = filteredCandidatesWith0s[i];
} return (resultAssets, resultPrices);
} function _requireWhitelistedSybil(address sybil) internal view {
require(isSybilWhitelisted(sybil), 'INVALID_SYBIL');
}
}
The function submitPrices is guarded by the onlySybil modifier.
How audits missed it
To audit firms defense, PriceOracle.sol is located at mocks/oracle/PriceOracle.sol. This means this file should not be deployed in production, but rather local forks for testing purposes. We can verify this by looking at the test suite.
it('Get price of asset with no asset source', async () => {
const { aaveOracle, oracle } = testEnv;
const fallbackPrice = oneEther; // Register price on FallbackOracle
expect(await oracle.setAssetPrice(mockToken.address, fallbackPrice)); // Asset has no source
expect(await aaveOracle.getSourceOfAsset(mockToken.address)).to.be.eq(ZERO_ADDRESS); // Returns 0 price
expect(await aaveOracle.getAssetPrice(mockToken.address)).to.be.eq(fallbackPrice);
});
We suspect Aave deployed the contracts in production by copying the test suite. We cannot prove this claim because the deployment scripts are not public.
Having said that, this issue exposes limitations of security audit. Audits are usually done before contracts are deployed on the mainnet. If an issue lies in the deployment scripts, unless being in scope, they won’t be caught. To achieve maximum security, the deployment process should also be audited, preferably before and after deployment.
Fixes and recommendations
Aave fixed the issue on all supported chains quickly once they were notified of the issue. The fix involves disabling the fallback oracle, by setting the _fallbackOracle to address 0x0, as can be seen from https://polygonscan.com/tx/0x194abde14ec6baaabbdff3ef7973a70c70e109021859091bc90abe0970fc34d7.
Protocols should have their contract configurations audited after the contracts are deployed. Besides, any new contracts should also be audited before being put online. The fact that this issue had gone unnoticed for a month implies that no one cares once audits are performed, as can also seen in the recent Beanstalk hack that resulted in a $182M loss.
Timeline
April 7th — We contacted Aave via security@aave.com
April 19th — We reached out to @samczsun on Telegram after not hearing back from Aave
April 19th — Aave was notified of the issue
April 20th — Issue was fixed; Bounty pending
July 12th— The governance proposal of awarding us a $50,000 bounty passed
We are Hacxyk, we provide an exceedingly thorough analysis of a blockchain application’s smart contracts in order to correct design issues, errors in the code, or identify security vulnerabilities. We perform both manual analysis and automated testing to make sure your smart contract application or DeFi platform is ready for mainnet.
Let us know if you are interested in getting an audit for your project.
Find us at audit[at]hacxyk.com or @hacxyk on Telegram
