Users Can Bypass `borrowATokenCap` Check Through `multicall` Due to Incorrect Validation

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/Multicall.sol#L29-L42 https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/CapsLibrary.sol#L19-L44

Vulnerability details

• The borrowATokenCap in the Size protocol is a mechanism to limit the total supply of borrowAToken that can be minted, meaning the maximum amount that can be deposited. This cap is crucial for controlling the expansion of credit within the system and ensuring that the protocol grows within manageable risk parameters.

• When a user deposits in the protocol, there is always a validation that checks if this deposit will lead to exceeding the borrowATokenCap, and if so, it reverts.

   function executeDeposit(State storage state, DepositParams calldata params) public {
  
      // some code ... 
  
      if (params.token == address(state.data.underlyingBorrowToken)) {
          state.depositUnderlyingBorrowTokenToVariablePool(from, params.to, amount);
          // borrow aToken cap is not validated in multicall,
          //   since users must be able to deposit more tokens to repay debt
          if (!state.data.isMulticall) {
  >>            state.validateBorrowATokenCap();
          }
      } 
      // more code ...
  }
  
  function validateBorrowATokenCap(State storage state) external view {
  >>    if (state.data.borrowAToken.totalSupply() > state.riskConfig.borrowATokenCap) {
  >>       revert Errors.BORROW_ATOKEN_CAP_EXCEEDED(state.riskConfig.borrowATokenCap, state.data.borrowAToken.totalSupply());
      }
  }

• Notice that in the case of multicall, this invariant is not checked. This is because the protocol allows exceeding the borrowATokenCap if the deposited tokens are meant to be used in liquidation or to repay a debt, which is crucial to maintain the protocol's health. Instead, in multicall, we check that the exceedance of borrowATokenCap is indeed used to reduce debt and not for a regular deposit.
• The issue is that we take the balance of the Size contract of borrowAToken instead of borrowAToken.totalSupply() to calculate the excess. This is incorrect because if a user is depositing, the borrowAToken amount deposited will be credited to that user and not to the Size contract.

    function multicall(State storage state, bytes[] calldata data) internal returns (bytes[] memory results) {
        state.data.isMulticall = true;

  >>    uint256 borrowATokenSupplyBefore = state.data.borrowAToken.balanceOf(address(this));
  >>    uint256 debtTokenSupplyBefore = state.data.debtToken.totalSupply();

        results = new bytes[](data.length);
        for (uint256 i = 0; i < data.length; i++) {
            results[i] = Address.functionDelegateCall(address(this), data[i]);
        }
        uint256 borrowATokenSupplyAfter = state.data.borrowAToken.balanceOf(address(this));
        uint256 debtTokenSupplyAfter = state.data.debtToken.totalSupply();

  >>    state.validateBorrowATokenIncreaseLteDebtTokenDecrease(borrowATokenSupplyBefore, debtTokenSupplyBefore, borrowATokenSupplyAfter, debtTokenSupplyAfter);
        state.data.isMulticall = false;
    }

    function validateBorrowATokenIncreaseLteDebtTokenDecrease(
        State storage state,
        uint256 borrowATokenSupplyBefore,
        uint256 debtTokenSupplyBefore,
        uint256 borrowATokenSupplyAfter,
        uint256 debtTokenSupplyAfter
    ) external view {
        // If the supply is above the cap
 >>     if (borrowATokenSupplyAfter > state.riskConfig.borrowATokenCap) {
            uint256 borrowATokenSupplyIncrease = borrowATokenSupplyAfter > borrowATokenSupplyBefore
                ? borrowATokenSupplyAfter - borrowATokenSupplyBefore
                : 0;
            uint256 debtATokenSupplyDecrease =
                debtTokenSupplyBefore > debtTokenSupplyAfter ? debtTokenSupplyBefore - debtTokenSupplyAfter : 0;
            // and the supply increase is greater than the debt reduction
 >>         if (borrowATokenSupplyIncrease > debtATokenSupplyDecrease) {
 >>             revert Errors.BORROW_ATOKEN_INCREASE_EXCEEDS_DEBT_TOKEN_DECREASE(borrowATokenSupplyIncrease, debtATokenSupplyDecrease);
            }
            // otherwise, it means the debt reduction was greater than the inflow of cash: do not revert
        }
        // otherwise, the supply is below the cap: do not revert
    }

• This allows any user to deposit any amount that exceeds the borrowATokenCap through multicall, breaking this functionality.

  Impact

  • Users can always exceed the borrowATokenCap via multicall, breaking this functionality and potentially exposing the protocol to financial risks.

    Proof of Concept

• here a coded poc that shows how alice was able to deposit an amount that exceeds borrowATokenCap through multicall where the deposited amount not used to repay or liquidate a debt, add this test here

   function test_ExceedCap_Poc() public {
      _setPrice(1e18);
      uint256 amount = 100e6;
      uint256 cap = amount;
      _updateConfig("borrowATokenCap", cap);
  
      _deposit(alice, usdc, cap);
      _deposit(bob, weth, 200e18);
      // credit alice more token :
      _mint(address(usdc), alice, 10000e6);
      _approve(alice, address(usdc), address(size), 10000e6);
  
      // encode data for multicall to deposit :
      DepositParams memory params = DepositParams({token: address(usdc), amount: 10000e6, to: alice});
      bytes[] memory depositData = new bytes[](1);
      depositData[0] = abi.encodeWithSelector(size.deposit.selector, params);
      uint256 borrowATokenBefore = size.getUserView(alice).borrowATokenBalance;
  
      // deposit will fail due reaching cap with normal deposit :
      vm.expectRevert(abi.encodeWithSelector(Errors.BORROW_ATOKEN_CAP_EXCEEDED.selector, cap, 100e6 + 10000e6));
      vm.prank(alice);
      size.deposit(DepositParams({token: address(usdc), amount: 10000e6, to: alice}));
      // alice can buy pass this by depositing through multicall :
      vm.prank(alice);
      size.multicall(depositData);
      uint256 borrowATokenAfter = size.getUserView(alice).borrowATokenBalance;
      assertTrue(borrowATokenBefore == 100e6 && borrowATokenAfter == 10000e6 + 100e6);
  }

  Tools Used

  manual review , foundry testing

  Recommended Mitigation Steps

• in multicall use totalSupply instead of balanceOf(address(this)) :

  
  function multicall(State storage state, bytes[] calldata data) internal returns (bytes[] memory results) {
      state.data.isMulticall = true;

• uint256 borrowATokenSupplyBefore = state.data.borrowAToken.balanceOf(address(this));

• uint256 borrowATokenSupplyBefore = state.data.borrowAToken.totalSupply() uint256 debtTokenSupplyBefore = state.data.debtToken.totalSupply();

  results = new bytes[](data.length);
  for (uint256 i = 0; i < data.length; i++) {
      results[i] = Address.functionDelegateCall(address(this), data[i]);
  }

• uint256 borrowATokenSupplyAfter = state.data.borrowAToken.balanceOf(address(this));

• uint256 borrowATokenSupplyAfter = state.data.borrowAToken.totalSupply() uint256 debtTokenSupplyAfter = state.data.debtToken.totalSupply();

  state.validateBorrowATokenIncreaseLteDebtTokenDecrease(borrowATokenSupplyBefore, debtTokenSupplyBefore, borrowATokenSupplyAfter, debtTokenSupplyAfter);
  state.data.isMulticall = false;

  }

Assessed type

Invalid Validation

[c4-judge]

hansfriese marked the issue as duplicate of #144

[c4-judge]

hansfriese marked the issue as satisfactory

[c4-judge]

hansfriese marked the issue as duplicate of #238