Incorrect Validation of Amount in sellCreditMarket and buyCreditMarket Functions Causes A Revert On Valid Inputs

[howlbot-integration]

Lines of code

https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/actions/SellCreditMarket.sol#L93-L94
https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/actions/BuyCreditMarket.sol#L91-L92

Vulnerability details

Impact

• Users with valid input will get a revert in this case due to incorrect validation, rendering the exactAmountIn mechanism not working as intended.

Proof of Concept

• When a user tries to buy or sell a credit, they call either the sellCreditMarket or buyCreditMarket function. Both functions validate the user's input before executing the action.
• The user specifies an Amount and a boolean exactAmountIn, which indicates whether the Amount represents credit or cash.
• During validation, both functions compare the Amount with minimumCreditBorrowAToken which is the minimum credit allowed and revert if Amount < minimumCreditBorrowAToken.

 function validateSellCreditMarket(State storage state, SellCreditMarketParams calldata params) external view {
      // some code ...

>>        if (params.amount < state.riskConfig.minimumCreditBorrowAToken) {
           revert Errors.CREDIT_LOWER_THAN_MINIMUM_CREDIT(params.amount, state.riskConfig.minimumCreditBorrowAToken);
       }
    }

   function validateBuyCreditMarket(State storage state, BuyCreditMarketParams calldata params) external view {
      
>>     if (params.amount < state.riskConfig.minimumCreditBorrowAToken) {
           revert Errors.CREDIT_LOWER_THAN_MINIMUM_CREDIT(params.amount, state.riskConfig.minimumCreditBorrowAToken);
       }
    }

• The issue arises when the Amount is intended to represent cashAmount rather than credit. In such cases, even if the Amount is less than minimumCreditBorrowAToken, applying the fees and borrow rate to calculate the credit might result in a credit amount greater than minimumCreditBorrowAToken. Therefore, the validation is incorrect when the Amount represents cash. which broke those invariants :

DOS: Functions should not revert if preconditions are met (Denial of Service)
REVERTS: Actions behave as expected under dependency reverts

Example:

• minimumCreditBorrowAToken = 100, ratePerTenor = 20%, swapFees = 0.5%, Amount = 95

• If we calculate credit for 95 cash, we get:
  credit = (95 + 95 * 0.005) * 1.2 = 114.57

• Notice that the credit is more than minimumCreditBorrowAToken, but the transaction still reverts due to the incorrect comparison (95 < 100).

Tools Used

• manual review

Recommended Mitigation Steps

• There is no need to check the minimum amount of credit in the validation function it's a waste of gas, as it always get checked in the execute function anyway here:

    function executeBuyCreditMarket(State storage state, BuyCreditMarketParams memory params) external returns (uint256 cashAmountIn) {
        

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

    function executeSellCreditMarket(State storage state, SellCreditMarketParams calldata params) external returns (uint256 cashAmountOut) {
        

        if (params.creditPositionId == RESERVED_ID) {
  >>          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
        });
        // more code 
    }

• remove the check for minimumCreditBorrowAToken in both function :

  function validateSellCreditMarket(State storage state, SellCreditMarketParams calldata params) external view {
       // some code ...

--        if (params.amount > creditPosition.credit) {
--              revert Errors.NOT_ENOUGH_CREDIT(params.amount, creditPosition.credit);
--          }
   }

    function validateBuyCreditMarket(State storage state, BuyCreditMarketParams calldata params) external view {
        // some code ....
       
--     if (params.amount < state.riskConfig.minimumCreditBorrowAToken) {
--          revert Errors.CREDIT_LOWER_THAN_MINIMUM_CREDIT(params.amount, state.riskConfig.minimumCreditBorrowAToken);
--      }
     }

Assessed type

Invalid Validation

[c4-judge]

hansfriese marked the issue as satisfactory