Skip to content
This repository has been archived by the owner on Dec 17, 2023. It is now read-only.

0x52 - eMode implementation is completely broken #251

Open
sherlock-admin opened this issue Jun 14, 2023 · 4 comments
Open

0x52 - eMode implementation is completely broken #251

sherlock-admin opened this issue Jun 14, 2023 · 4 comments
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability High A valid High severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin
Copy link
Contributor

0x52

high

eMode implementation is completely broken

Summary

Enabling eMode allows assets of the same class to be borrowed at much higher a much higher LTV. The issue is that the current implementation makes the incorrect calls to the Aave V3 pool making so that the pool can never take advantage of this higher LTV.

Vulnerability Detail

AaveLeverageStrategyExtension.sol#L1095-L1109

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);

When calculating the max borrow/repay allowed, the contract uses the getReserveConfigurationData subcall to the pool.

AaveProtocolDataProvider.sol#L77-L100

function getReserveConfigurationData(
  address asset
)
  external
  view
  override
  returns (
      ...
  )
{
  DataTypes.ReserveConfigurationMap memory configuration = IPool(ADDRESSES_PROVIDER.getPool())
    .getConfiguration(asset);

  (ltv, liquidationThreshold, liquidationBonus, decimals, reserveFactor, ) = configuration
    .getParams();

The issue with using getReserveConfigurationData is that it always returns the default settings of the pool. It never returns the adjusted eMode settings. This means that no matter the eMode status of the set token, it will never be able to borrow to that limit due to calling the incorrect function.

It is also worth considering that the set token as well as other integrated modules configurations/settings would assume this higher LTV. Due to this mismatch, the set token would almost guaranteed be misconfigured which would lead to highly dangerous/erratic behavior from both the set and it's integrated modules. Due to this I believe that a high severity is appropriate.

Impact

Usage of eMode, a core function of the contracts, is completely unusable causing erratic/dangerous behavior

Code Snippet

AaveLeverageStrategyExtension.sol#L1095-L1109

Tool used

Manual Review

Recommendation

Pull the adjusted eMode settings rather than the base pool settings

@github-actions github-actions bot added the High A valid High severity issue label Jun 19, 2023
@ckoopmann ckoopmann added the Sponsor Confirmed The sponsor acknowledged this issue is valid label Jun 20, 2023
@ckoopmann
Copy link

Yep, this is correct, and will be addressed.

Btw: I think I saw a bunch of duplicates of this issue when looking through the unfiltered list.

@0xffff11
Copy link
Collaborator

Agree with sponsor, valid high. Added missing duplicates

@ckoopmann
Copy link

ckoopmann commented Jul 4, 2023

Fixed in below pr by keeping track of the current eMode category id and then getting the data for that specific eMode (if eMode category is not 0):
IndexCoop/index-coop-smart-contracts#142

@IAm0x52
Copy link
Collaborator

IAm0x52 commented Aug 1, 2023

Fix looks good. If emode is activated then it will use emode ltv and liquidation threshold

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability High A valid High severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

4 participants