Bauchibred - Protocol doesn't completely protect itself from `LTV = 0` tokens
Opened this issue · 5 comments
Bauchibred
high
Protocol doesn't completely protect itself from LTV = 0 tokens
Summary
The AaveLeverageStrategyExtension does not completely protect against tokens with a Loan-to-Value (LTV) of 0. Tokens with an LTV of 0 in Aave V3 pose significant risks, as they cannot be used as collateral to borrow upon a breaking withdraw. Moreover, LTVs of assets could be set to 0, even though they currently aren't, it could create substantial problems with potential disruption of multiple functionalities. This bug could cause a Denial-of-Service (DoS) situation in some cases, and has a potential to impact the borrowing logic in the protocol, leading to an unintentionally large perceived borrowing limit.
Vulnerability Detail
When an AToken has LTV = 0, Aave restricts the usage of certain operations. Specifically, if a user owns at least one AToken as collateral with an LTV = 0, certain operations could revert:
- Withdraw: If the asset being withdrawn is collateral and the user is borrowing something, the operation will revert if the withdrawn collateral is an AToken with LTV > 0.
- Transfer: If the asset being transferred is an AToken with LTV > 0 and the sender is using the asset as collateral and is borrowing something, the operation will revert.
- Set the reserve of an AToken as non-collateral: If the AToken being set as non-collateral is an AToken with LTV > 0, the operation will revert.
Take a look at AaveLeverageStrategyExtension.sol#L1050-L1119
/**
* Calculate total notional rebalance quantity and chunked rebalance quantity in collateral units.
*
* return uint256 Chunked rebalance notional in collateral units
* return uint256 Total rebalance notional in collateral units
*/
function _calculateChunkRebalanceNotional(
LeverageInfo memory _leverageInfo,
uint256 _newLeverageRatio,
bool _isLever
)
internal
view
returns (uint256, uint256)
{
// Calculate absolute value of difference between new and current leverage ratio
uint256 leverageRatioDifference = _isLever ? _newLeverageRatio.sub(_leverageInfo.currentLeverageRatio) : _leverageInfo.currentLeverageRatio.sub(_newLeverageRatio);
uint256 totalRebalanceNotional = leverageRatioDifference.preciseDiv(_leverageInfo.currentLeverageRatio).preciseMul(_leverageInfo.action.collateralBalance);
uint256 maxBorrow = _calculateMaxBorrowCollateral(_leverageInfo.action, _isLever);
uint256 chunkRebalanceNotional = Math.min(Math.min(maxBorrow, totalRebalanceNotional), _leverageInfo.twapMaxTradeSize);
return (chunkRebalanceNotional, totalRebalanceNotional);
}
/**
* Calculate the max borrow / repay amount allowed in base units for lever / delever. This is due to overcollateralization requirements on
* assets deposited in lending protocols for borrowing.
*
* For lever, max borrow is calculated as:
* (Net borrow limit in USD - existing borrow value in USD) / collateral asset price adjusted for decimals
*
* For delever, max repay is calculated as:
* Collateral balance in base units * (net borrow limit in USD - existing borrow value in USD) / net borrow limit in USD
*
* Net borrow limit for levering is calculated as:
* The collateral value in USD * Aave collateral factor * (1 - unutilized leverage %)
*
* Net repay limit for delevering is calculated as:
* The collateral value in USD * Aave liquiditon threshold * (1 - unutilized leverage %)
*
* return uint256 Max borrow notional denominated in collateral asset
*/
function _calculateMaxBorrowCollateral(ActionInfo memory _actionInfo, bool _isLever) internal view returns(uint256) {
// Retrieve collateral factor and liquidation threshold for the collateral asset in precise units (1e16 = 1%)
( , uint256 maxLtvRaw, uint256 liquidationThresholdRaw, , , , , , ,) = strategy.aaveProtocolDataProvider.getReserveConfigurationData(address(strategy.collateralAsset));
// Normalize LTV and liquidation threshold to precise units. LTV is measured in 4 decimals in Aave which is why we must multiply by 1e14
// for example ETH has an LTV value of 8000 which represents 80%
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);
}
}Apart from the aforementioned issue with LTV = 0 tokens, there's another issue with the _calculateMaxBorrowCollateral() function. When LTV = 0, maxLtvRaw also equals 0, leading to a netBorrowLimit of 0. When the borrowing value is subtracted from this, it results in an underflow, causing the borrowing limit to appear incredibly large. This essentially breaks the borrowing logic of the protocol.
Impact
This bug could potentially disrupt the entire borrowing logic within the protocol by inflating the perceived borrowing limit. This could lead to users borrowing an unlimited amount of assets due to the underflow error. In extreme cases, this could lead to a potential loss of user funds or even a complete protocol shutdown, thus impacting user trust and the overall functionality of the protocol.
Code Snippet
AaveLeverageStrategyExtension.sol#L1050-L1119
Tool used
Manual Review
Recommendation
The protocol should consider implementing additional protections against tokens with an LTV of 0.
This raises a good point that wasn't fully considered during the design.
We will investigate this.
Generally I don't think the "Impact" section is accurate though as users cannot borrow assets on behalf of the token. However if the previous information is accurate it could have an affect on issuance / redemption if the aToken transfers are blocked.
I set this to confirmed as the issue raises valid questions / scenarios that weren't fully considered during the design.
Unfortunately the recommendation is pretty vague and not very actionable.
After digging into this audit report it seems that at least there are protections in place on aave side that no malicious user could produce such a situation by sending us aTokens with ltv=0 or something like that.
So the only scenario where this situation could arise would be if aave governance sets the LTV of the collateral token we are using to 0.
One potential change that could allow us to delever the token in such a situation could be to add flashloan based delevering as also suggested in this issue:
#255
Also see this issue on the morpho spearbit audit for reference. (You will have to create an account solodit to access it) - note that the "Vulernability Detail" section seems to be copy / pasted from that report:
https://solodit.xyz/issues/16216
After rereading this more carefully it seems the above listed limitations only affect a second aToken with LTV > 0. So for this scenario to come into effect we would have to have two aTokens as components in the set, one of which would have LTV = 0 and the other one (which would not be able to be transfered anymore) would have LTV > 0.
While the issue seems to be valid, if the above understanding is correct we might keep the logic as is and list this as an explicit limitation. Because the strategy extension is designed to work with only one aToken anyway. (The Set Token could have another aToken as a component that is not managed as part of the leverage strategy but this should be avoided and is certainly not part of the expected use)
While the issue seems to be valid, if the above understanding is correct we might keep the logic as is and list this as an explicit limitation
Keeping the issue as a med because there is still a small possibility for this to happen even though it is not the intended behavior:
he Set Token could have another aToken as a component that is not managed as part of the leverage strategy but this should be avoided and is certainly not part of the expected use