code-423n4 / 2024-06-size-findings

2 stars 0 forks source link

Protocol is not usable due to incorrect aaveV3 liquidity check #114

Closed c4-bot-8 closed 3 months ago

c4-bot-8 commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/Size.sol#L178 https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/Size.sol#L188 https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/Size.sol#L229

Vulnerability details

Impact

Protocol checks the current liquidity of the underlying borrow token in aaveV3 pool in buyCreditMarket(), sellCreditMarket() and liquidateWithReplacement(). The check gets the balance of an incorrect address (the variable pool) which does not hold any funds. This causes a revert every time any of these 3 functions are called.

Proof of Concept

Inside variablePool.supply() function inside AaveV3 code (that can be inspected via etherscan here) we can see that the funds are actually transferred immediately to the aTokenAddress:

function executeSupply(
    mapping(address => DataTypes.ReserveData) storage reservesData,
    mapping(uint256 => address) storage reservesList,
    DataTypes.UserConfigurationMap storage userConfig,
    DataTypes.ExecuteSupplyParams memory params
  ) external {
    DataTypes.ReserveData storage reserve = reservesData[params.asset];
    DataTypes.ReserveCache memory reserveCache = reserve.cache();

    reserve.updateState(reserveCache);

    ValidationLogic.validateSupply(reserveCache, reserve, params.amount);

    reserve.updateInterestRates(reserveCache, params.asset, params.amount, 0);

@>  IERC20(params.asset).safeTransferFrom(msg.sender, reserveCache.aTokenAddress, params.amount);

    bool isFirstSupply = IAToken(reserveCache.aTokenAddress).mint(
      msg.sender,
      params.onBehalfOf,
      params.amount,
      reserveCache.nextLiquidityIndex
    );

    if (isFirstSupply) {
      if (
        ValidationLogic.validateAutomaticUseAsCollateral(
          reservesData,
          reservesList,
          userConfig,
          reserveCache.reserveConfiguration,
          reserveCache.aTokenAddress
        )
      ) {
        userConfig.setUsingAsCollateral(reserve.id, true);
        emit ReserveUsedAsCollateralEnabled(params.asset, params.onBehalfOf);
      }
    }

    emit Supply(params.asset, msg.sender, params.onBehalfOf, params.amount, params.referralCode);
  }

The check will revert every time since the balance of variable pool is going to be 0.

Link to code: link

    function validateVariablePoolHasEnoughLiquidity(State storage state, uint256 amount) public view {
@>        uint256 liquidity = state.data.underlyingBorrowToken.balanceOf(address(state.data.variablePool));
@>      if (liquidity < amount) {
            revert Errors.NOT_ENOUGH_BORROW_ATOKEN_LIQUIDITY(liquidity, amount);
        }
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Use the aToken address to check what is the current liquidity:

    function validateVariablePoolHasEnoughLiquidity(State storage state, uint256 amount) public view {
-         uint256 liquidity = state.data.underlyingBorrowToken.balanceOf(address(state.data.variablePool));
+         address aToken = state.data.variablePool.getReserveData(address(state.data.underlyingBorrowToken)).aTokenAddress;
+         uint256 liquidity = state.data.underlyingBorrowToken.balanceOf(aToken);

        if (liquidity < amount) {
            revert Errors.NOT_ENOUGH_BORROW_ATOKEN_LIQUIDITY(liquidity, amount);
        }
    }

Assessed type

Invalid Validation

aviggiano commented 3 months ago

Fixed in https://github.com/SizeCredit/size-solidity/pull/127

C4-Staff commented 3 months ago

CloudEllie marked the issue as duplicate of #218

c4-judge commented 3 months ago

hansfriese marked the issue as satisfactory

c4-judge commented 2 months ago

hansfriese changed the severity to 2 (Med Risk)