Using multicall functionality, one can bypass the deposit cap even without reducing debt

[c4-bot-8]

Lines of code

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

Vulnerability details

The function Size::multicall allows users to batch multiple function calls in one external call. Among other things, multicall functionality allows going over the underlying borrow token (USDC currently) deposit limit only if it is to repay debt in the system therefore increasing the system’s health.

As can be seen here, multicalls are not checked for breaching the underlying borrow token cap. The reason for this is that the protocol wants to allow users to deposit USDC only if they are being used to decrease the debt in the protocol (through regular borrower repayments or through liquidations). This reason justifies the breaching of the cap as can bee seen in this comment. If not for multicall, users will need to first call Size::deposit and then another external call for Size::repay (or one of the liquidation functions) in order to repay debt. But if depositing breaches the cap then it will revert, therefore not allow users to repay debts once the cap has been reached. The multicall allows for bypassing the cap check so that users can deposit and repay.

We can see in the function Multicall::multicall that the function will read the borrow aToken balance of the Size contract before and after the multicall, as well as the debt token total supply before and after. Then it will send this data to CapsLibrary::validateBorrowATokenIncreaseLteDebtTokenDecrease to make sure the increase in borrow tokens is equal or smaller than the reduction in debt tokens, as can be seen in this check.

Essentially what the protocol wants to achieve is to allow users to deposit an amount of USDC, even if the deposit cap has been reached, only to be used for repaying some debt. If depositing the USDC breaches the cap and the reduction in debt tokens is equal or greater than the deposited amount, then the protocol will allow breaching of the cap in exchange for a healthier protocol.

The issue here is that the way that Multicall::multicall and CapsLibrary::validateBorrowATokenIncreaseLteDebtTokenDecrease are currently implemented, they allow users to deposit through the multicall functionality (instead of the regular deposit flow via Size::deposit) and breach the cap without repaying any debt. Meaning, users can use multicall to deposit tokens, with no other function call batched with the deposit, and the validation function will not revert.

The root cause is in Multicall:multicall where the function reads the value of the balance of borrow aUSDC tokens held by Size before and after the multicall. While it is true that the balance of Size increases when users repay debt (since the repayment is transferred from msg.sender to Size), it doesn’t change when users just deposit, since the borrow tokens are assigned to the user that deposited and not to Size. This will allow this check to be bypassed since borrowATokenSupplyIncrease is 0 (balance of Size before and after does not change). Instead, the function should rely on total supply of borrow aUSDC instead of Size’s balance.

Impact

The borrow aToken (szaUSDC) cap can be breached despite the fact that the protocol’s intended design is to allow it to be breached only to increase the health of the system (reducing debt). By breaching the cap freely, users can increase the USDC (and szaUSDC) total supply regardless of the cap enforced, without increasing the health of the system, therefore increasing the risk taken by the protocol as unlimited funds are flowing into it.

Proof of Concept

Add the following test function into MulticallTest:

function test_Multicall_multicall_bypasses_cap_just_by_depositing_without_reducing_debt() public {
        _setPrice(1e18);
        uint256 amount = 100e6;
        uint256 cap = amount + size.getSwapFee(100e6, 365 days);
        _updateConfig("borrowATokenCap", cap);

        _deposit(alice, usdc, cap);
        _deposit(bob, weth, 200e18);

        _buyCreditLimit(alice, block.timestamp + 365 days, YieldCurveHelper.pointCurve(365 days, 0.1e18));
        uint256 debtPositionId = _sellCreditMarket(bob, alice, RESERVED_ID, amount, 365 days, false);
        uint256 futureValue = size.getDebtPosition(debtPositionId).futureValue;

        vm.warp(block.timestamp + 365 days);

        assertEq(_state().bob.debtBalance, futureValue);

        uint256 remaining = futureValue - size.getUserView(bob).borrowATokenBalance;
        _mint(address(usdc), bob, remaining);
        _approve(bob, address(usdc), address(size), remaining);

        // attempt to deposit to repay, but it reverts due to cap
        vm.expectRevert(abi.encodeWithSelector(Errors.BORROW_ATOKEN_CAP_EXCEEDED.selector, cap, cap + remaining));
        vm.prank(bob);
        size.deposit(DepositParams({token: address(usdc), amount: remaining, to: bob}));

        assertEq(_state().bob.debtBalance, futureValue);

        // cap is breached just by using the multicall functionality without actually reducing any debt
        bytes[] memory data = new bytes[](1);
        data[0] = abi.encodeCall(size.deposit, DepositParams({token: address(usdc), amount: remaining, to: bob}));
        vm.prank(bob);
        size.multicall(data);

	// Bob's debt hasn't changed
        assertEq(_state().bob.debtBalance, futureValue);
    }

Note that this function is essentially the same as MulticallTest::test_Multicall_multicall_bypasses_cap_if_it_is_to_reduce_debt, but without data[1] which encodes a repayment function. This shows that calling Size::deposit will revert when the deposit goes over the cap, but calling the same deposit via multicall will not revert and will breach the cap.

Tools Used

Manual review

Recommended Mitigation Steps

Make the following changes in Multicall::multicall

...

- uint256 borrowATokenSupplyBefore = state.data.borrowAToken.balanceOf(address(this));
+ uint256 borrowATokenSupplyBefore = state.data.borrowAToken.totalSupply();

...

- uint256 borrowATokenSupplyAfter = state.data.borrowAToken.balanceOf(address(this));
+ uint256 borrowATokenSupplyAfter = state.data.borrowAToken.totalSupply();

Assessed type

Invalid Validation

[c4-judge]

hansfriese marked the issue as satisfactory

[c4-judge]

hansfriese marked the issue as duplicate of #238