AToken cap can be violated/bypassed through multicall

[howlbot-integration]

Lines of code

https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/actions/Deposit.sol#L76-L82
https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/Multicall.sol#L29-L42

Vulnerability details

Impact

The borowATokenCap which is meant to limit the amount of credit in the protocol can be bypassed by depositing via multicall

Proof of Concept

The borrowATokenCap check is deferred during a multicall to allow for loan repayments and liquidations that might result in the total borrowAToken exceeding the cap provided that an equal amount of debt is repaid in the same multicall. here

However, the validation logic used after the multicall is incorrect as it allows a user to deposit tokens that might exceed the cap and would revert if done through a direct deposit call.

The AToken cap validation function used in multicall validateBorrowATokenIncreaseLteDebtTokenDecrease asserts that if the borrowATokenCap is exceeded the an equal or greater amount of debt must be burned, as stated earlier this is to allow loan repayments and liquidations that might excced the cap. The issue lies in the borrowATokenSupplyBefore and borrowATokenSupplyAfter parameters passed to the validateBorrowATokenIncreaseLteDebtTokenDecrease function.
As seen here the balanceOf(address(this)) is used rather than totalSupply(), since deposits have no effect on balanceOf(address(this)) as tokens is deposited directly to the user's balance, the validation fails.

Proof of Code

Lets extend the existing deposit validation test by adding the following lines of code

// /test/local/actions/DepositValidation.t.sol

    function test_Deposit_validation_borrowATokenCap() public {
        uint256 amount = 4_000_000;
        _mint(address(weth), alice, amount * 1e18);
        _mint(address(usdc), alice, amount * 1e6);
        _approve(alice, address(weth), address(size), amount * 1e18);
        _approve(alice, address(usdc), address(size), amount * 1e6);

        vm.startPrank(alice);
        size.deposit(DepositParams({token: address(weth), amount: amount * 1e18, to: alice}));

        vm.expectRevert(
            abi.encodeWithSelector(
                Errors.BORROW_ATOKEN_CAP_EXCEEDED.selector, size.riskConfig().borrowATokenCap, amount * 1e6
            )
        );
        size.deposit(DepositParams({token: address(usdc), amount: amount * 1e6, to: alice}));

        //deposit fails, user uses multicall to deposit anyways!.
->      bytes[] memory data = new bytes[](1);
->      data[0] = abi.encodeCall(size.deposit, DepositParams({token: address(usdc), amount: amount * 1e6, to: alice}));
->      size.multicall(data);
    }

Run via forge test --mt test_Deposit_validation_borrowATokenCap.
From the test code provided we can see that the deposit validation logic does not work as expected when depositing though a multicall.

Tools Used

Manual Review

Recommended Mitigation Steps

Replace balanceOf(address(this)) with totalSupply()

// src/libraries/Multicall.sol

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 changed the severity to 2 (Med Risk)

[c4-judge]

hansfriese marked the issue as satisfactory

[c4-judge]

hansfriese marked the issue as duplicate of #238