A lender can Bypassing the validateUserIsNotBelowOpeningLimitBorrowCR Check in BuyCreditMarket there breaking a main invariant

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/Size.sol#L181-L183 https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/Size.sol#L192

Vulnerability details

Impact

The issue lies in the protocol's logic for checking the Credit Ratio (CR) during the purchase of a credit position via the BuyCreditMarket function.

Specifically:

• Initial Design: The protocol intends to ensure that any credit position purchase has a CR above the opening limit (e.g., 1.5), which is well above the liquidation threshold (1.3).
• Issue: The current implementation only checks the CR if a specific reserved credit position ID (RESERVED_ID) is used. However, when a user buys an existing credit position, the creditPositionId is set to the old credit position's ID/creates a new one. This allows the user to bypass the CR check and purchase a credit position with a CR close to the liquidation threshold, thus exploiting the system for potential profit by aiming to liquidate soon after purchase.

Here's the vulnerability that can be exploited:

• Code Snippet:

  if (params.creditPositionId == RESERVED_ID) {
    state.validateUserIsNotBelowOpeningLimitBorrowCR(params.borrower);
  }

Proof of Concept

Exploitation Scenario:

1. Setup: Assume the protocol has a CR opening limit set to 1.5(Based on the test setup in test).
2. User Action: A user purchases an existing credit position with a CR close to the liquidation threshold (1.3).

3. Outcome: Since the creditPositionId is not RESERVED_ID, the check validateUserIsNotBelowOpeningLimitBorrowCR is bypassed, allowing the user to purchase a position with a dangerously low CR.

   Potential Consequences

• Financial Exploitation: Users/Bot/Exploiters can intentionally purchase credit positions close to liquidation to profit quickly, without adhering to the protocol's intended risk management.

Code Reference:

• BuyCreditMarket Function:

  function buyCreditMarket(uint256 creditPositionId, address buyer, uint256 amount) external {
    // Validate CR only for new positions, not for existing ones.
    if (creditPositionId == RESERVED_ID) {
        state.validateUserIsNotBelowOpeningLimitBorrowCR(buyer);
    }
  
    // Other logic for buying the credit position
  }

Test Case:

1. Setup: Set up a scenario where the opening CR limit is 1.5 and liquidation Ration 1.3.
2. User Action: Create a credit position with a CR of 1.3.
3. Purchase: Attempt to purchase this position using the buyCreditMarket function.
4. Expected Result: The purchase should succeed despite the CR being below the opening limit.

Tool Used

Manual Code Analysis

Recommended Mitigation Steps

Implement Comprehensive CR Check:

• Ensure that the CR is checked for both new and existing credit positions. Update the function to always validate the CR, regardless of the credit position ID.

  Example Code Update:

  function buyCreditMarket(uint256 creditPositionId, address buyer, uint256 amount) external {
     // Always validate CR
     state.validateUserIsNotBelowOpeningLimitBorrowCR(params.borrower);
  
     // Proceed with buying the credit position
  }

Assessed type

Error

[Tomiwasa0]

Thank you for judging @hansfriese,

In Sell Credit Market a Lender Can sell Credit in 2 ways,

1. With Reserve Id - Checks BorrowCR
2. with Credit Id - Uses Future credit to get funds like in compensate (CR doesn't matter)

Note in Buy credit there is no If and else statement nested so we don't need to check both as already stated above.

 if (params.creditPositionId == RESERVED_ID) {
            // slither-disable-next-line unused-return
            state.createDebtAndCreditPositions({
                lender: msg.sender,
                borrower: msg.sender,
                futureValue: creditAmountIn,
                dueDate: block.timestamp + tenor
            });
        }

        state.createCreditPosition({
            exitCreditPositionId: params.creditPositionId == RESERVED_ID
                ? state.data.nextCreditPositionId - 1
                : params.creditPositionId,
            lender: params.lender,
            credit: creditAmountIn
        });

In Buy Credit Market a Lender Can buy Credit in 2 ways,

1. With Reserve Id - Checks BorrowCR

2. with Credit Id - Bypass Check (eventhough we are buying, we can buy below Opening Borrow CR )

   if (params.creditPositionId == RESERVED_ID) {
   // slither-disable-next-line unused-return
   state.createDebtAndCreditPositions({
       lender: msg.sender,
       borrower: borrower,
       futureValue: creditAmountOut,
       dueDate: block.timestamp + tenor
   });
   } else {
   state.createCreditPosition({
       exitCreditPositionId: params.creditPositionId,
       lender: msg.sender,
       credit: creditAmountOut
   });
   }

   We should check the OpeningBorrowCr always in BUYcreditMarket regardless of what was used (Credit/Reserve Id). The below allows for a bypass when credit is used

   if (params.creditPositionId == RESERVED_ID) {
         state.validateUserIsNotBelowOpeningLimitBorrowCR(params.borrower);
       }