Anyone can bypass ```validateBorrowATokenCap``` check and deposit over the limit calling ```deposit()``` from multicall

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/actions/Deposit.sol#L80

Vulnerability details

Description

As stated in the docs of Size, protocol wants to have a "guarded launch" and prevent growing at a faster pace than its security/safety/trust. For this reason, they have implemented an initial cap on the amount of szaUSDC that can be minted which is 1,000,000. This check is happening in every deposit call which tries to deposit USDC but it is not happening if the isMulticall flag is true.

function executeDeposit(State storage state, DepositParams calldata params) public {
        // ...
        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();
            }
        } else {
            state.depositUnderlyingCollateralToken(from, params.to, amount);
        }

        // ...
    }

Link to code

So to call deposit() from multicall and bypass the check, user can call multicall() function giving only one call to deposit() function with the amount over the limit in data bytes[] (see PoC below). As we can see there will be no check of the total supply of szaUSDC at all in this case :

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

Link to code

Impact

As a result, anyone can easily bypass the validateBorrowATokenCap check and deposit over whatever limit is set by the protocol. This can break any assumptions and intentions of the team for "guarded launch" or any other reason to limit the supply.

Proof of Concept

To understand the issue, add the following test in Multicall.t.sol file and run forge test --mt test_Multicall_multicall_can_break_cap -vvvv :

    function test_Multicall_multicall_can_break_cap() public {
        vm.startPrank(alice);
        uint256 amount = 10_000_000e6;
        address token = address(usdc);
        deal(token, alice, amount);
        IERC20Metadata(token).approve(address(size), amount);

        assertEq(size.getUserView(alice).borrowATokenBalance, 0);

        bytes[] memory data = new bytes[](1);
        data[0] = abi.encodeCall(size.deposit, (DepositParams({token: token, amount: amount, to: alice})));
        bytes[] memory results = size.multicall(data);

        assertEq(results[0], bytes(""));

        assertEq(size.getUserView(alice).borrowATokenBalance, amount);
    }

Tools Used

Manual Review, Foundry

Recommended Mitigation Steps

Consider adding the check of the total supply of szaUSDC in the multicall() function as well.

    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.validateBorrowATokenCap();

        state.data.isMulticall = false;
    }

Assessed type

Context

[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