hildingr - Oracle Price miss matched when E-mode uses single oracle
Opened this issue · 14 comments
hildingr
medium
Oracle Price miss matched when E-mode uses single oracle
Summary
AAVE3 can turn on single oracle use on any E-mode category. When that is done collateral and the borrowed assets will be valued based on a single oracle price. When this is done the prices used in AaveLeverageStrategyExtension can differ from those used internally in AAVE3.
This can lead to an increased risk of liquidation and failures to re-balance properly.
Vulnerability Detail
There is currently no accounting for single oracle use in the AaveLeverageStragyExtension, if AAVE3 turns it on the extension will simply continue using its current oracles without accounting for the different prices.
When re-balancing the following code calculate the netBorrowLimit/netRepayLimit:
if (_isLever) {
uint256 netBorrowLimit = _actionInfo.collateralValue
.preciseMul(maxLtvRaw.mul(10 ** 14))
.preciseMul(PreciseUnitMath.preciseUnit().sub(execution.unutilizedLeveragePercentage));
return netBorrowLimit
.sub(_actionInfo.borrowValue)
.preciseDiv(_actionInfo.collateralPrice);
} else {
uint256 netRepayLimit = _actionInfo.collateralValue
.preciseMul(liquidationThresholdRaw.mul(10 ** 14))
.preciseMul(PreciseUnitMath.preciseUnit().sub(execution.unutilizedLeveragePercentage));
return _actionInfo.collateralBalance
.preciseMul(netRepayLimit.sub(_actionInfo.borrowValue))
.preciseDiv(netRepayLimit);
The _actionInfo.collateralValue and _adminInfo.borrowValue are _getAndValidateLeverageInfo() where they are both retrieved based on the current set chainlink oracle.
When E-mode uses a single oracle price a de-pegging of one of the assets will lead to incorrect values of netBorrowLimit and netRepayLimit depending on which asset is de-pegging.
collateralValue or borrowValue can be either larger or smaller than how they are valued internally in AAVE3.
Impact
When Levering
If collateralValue is to valued higher than internally in AAVE3 OR If borrowValue is to valued lower than internally in AAVE3:
The netBorrowLimit is larger than it should be we are essentially going to overriding execute.unutilizedLeveragePercentage and attempting to borrow more than we should.
If collateralValue is valued lower than internally in AAVE3 OR If borrowValue is to valued higher than internally in AAVE3:
The netBorrowLimit is smaller than it should be, we are not borrowing as much as we should. Levering up takes longer.
When Delevering
If collateralValue is to valued higher than internally in AAVE3 OR If borrowValue is to valued lower than internally in AAVE3:
We will withdraw more collateral and repay more than specified by execution.unutilizedLeveragePercentage.
If collateralValue is valued lower than internally in AAVE3 OR If borrowValue is to valued higher than internally in AAVE3:
We withdraw less and repay less debt than we should. This means that both ripcord() and disengage() are not functioning as they, they will not delever as fast they should. We can look at it as execution.unutilizedLeveragePercentage not being throttled.
The above consequences show that important functionality is not working as expected. "overriding" execution.unutilizedLeveragePercentage is a serious safety concern.
Code Snippet
Tool used
Manual Review
Recommendation
Aave3LeverageStrategyExtension should take single oracle usage into account. _calcualteMaxBorrowCollateral should check if there is a discrepancy and adjust such that the execute.unutilizedLeveragePercentage safety parameter is honored.
Escalate for 10 USDC
This issue is similar to #284 but is much more likely to happen in practice. If AAVE turns on single oracle for a category the oracles will be miss-configured this can lead to liquidation and other issues that I have outlined.
This is more likely to happen than #284 since single oracle is a feature of E-mode that can be turned on at any time for any E-mode category.
I want to escalate this to a valid Medium. I recommend that the index team implements functionality that take into account single oracle use which can be turned on at any point.
You've deleted an escalation for this issue.
Escalate for 10 USDC
This issue is similar to #284 but is much more likely to happen in practice. If AAVE turns on single oracle for a category the oracles will be miss-configured this can lead to liquidation and other issues that I have outlined.
This is more likely to happen than #284 since single oracle is a feature of E-mode that can be turned on at any time for any E-mode category.
I want to escalate this to a valid Medium. I recommend that the index team implements functionality that take into account single oracle use which can be turned on at any point.
Escalate for 10 USDC
This issue is similar to #284 but is much more likely to happen in practice. If AAVE turns on single oracle for a category the oracles will be miss-configured this can lead to liquidation and other issues that I have outlined.
This is more likely to happen than #284 since single oracle is a feature of E-mode that can be turned on at any time for any E-mode category.
I want to escalate this to a valid Medium. I recommend that the index team implements functionality that take into account single oracle use which can be turned on at any point.
You've created a valid escalation for 10 USDC!
To remove the escalation from consideration: Delete your comment.
You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.
Even using a single oracle in E-mode. Both implementations would still use chainlink right?
Yes, but the issue is that when single oracle is turned on one of the assets will be guaranteed to use a different oracle feed in the Index Protocol compared to AAVE. AAVE and Index will value the asset differently which means that the calculations in _calculateMaxBorrowCollateral will be incorrect.
Example:
Initial state: Both index and AAVE use feed 1 for X and feed 2 for Y. Assume E-mode is activated.
Single Oracle turned on: AAVE now uses feed 1 as the single oracle feed for the E-mode category, both X and Y will use feed 1.
Now Y follows feed 1 on AAVE and feed 2 on Index. The calculations in _calculateChunkRebalanceNotional will as a result be incorrect. The magnitude of the error depends on how much and which direction the oracle feeds diverge in.
Thanks! Seems reasonable to me and I could see the medium here. Thoughts? @IAm0x52
Same underlying divergence issue as #284 though this one definitely has a better example of how that would happen. My take is that these should be duped then presented to Index team as well as Sherlock for a final severity discussion.
In the unlikely case that something like this does occur (oracles are different AND diverge) the consequences are tremendous. Ultimately the severity would come down to the likelihood.
I disagree that this and #284 are duplicates. The issue I identify can happen during the expected behavior of AAVE V3 due to new functionality added in V3 that has not been accounted for in Index.
The likelihood is much larger since an Oracle feed mismatch is guaranteed to happen if Single Oracle is turned on. Single Oracle is a feature of AAVE V3, I therefore believe that it should not be considered an unlikely scenario to enter a state with mismatched feeds.
Even though it could be seen as an "external" / "admin" feature, this issue raises a valid concern.
Using the AaveOracle instead might acutally be the better choice and we will review internally if we want to make that fix / change.
Therefore I will mark this issue as "Sponsor confirmed".
While this issue seems to go into more detail of how oracle mismatch can happen, the core issue seems to be the same as 284.
Therefore this response also applies to both issues.
Even though it could be seen as an "external" / "admin" feature, this issue raises a valid concern. Using the AaveOracle instead might acutally be the better choice and we will review internally if we want to make that fix / change.
Therefore I will mark this issue as "Sponsor confirmed".
While this issue seems to go into more detail of how oracle mismatch can happen, the core issue seems to be the same as 284. Therefore this response also applies to both issues.
I agree that #284 is an admin/external since it is a potential issue that arise due to admin changing a parameter that is "trusted".
What I point to is an issue that arise due to index protocol not handling a feature added to AAVE V3. AAVE V3 is built to use E-modes where single oracle is turned on and off, any such event would put index at risk.
I think it is incorrect to label an issue an "admin" issue if it is a consequence of incompatibility with a feature of a protocol being used as expected.
Result:
Medium
Has duplicates
After further discussions internally and the protocol. Considering this issue a valid medium as changing parameters in the external protocol affects index protocol as shown in the issue.
Also considering issue #284 as a duplicate the underlying issue originates from the external protocol due to a change in parameters/functionality by the external admin, affecting oracle price.
More context on the external admin trust assumptions is updated in the judging guide.
https://docs.sherlock.xyz/audits/judging/judging#some-standards-observed
Fix looks good. Now using AAVE oracle so that oracles are always aligned