BuyCreditMarket and SellCreditMarket can revert despite using correct amount of credit that is more than minimumCreditBorrowAToken

[c4-bot-10]

Lines of code

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

Vulnerability details

Impact

User could try to create correct amount of credit (above minimumCreditBorrowAToken) but fail to do so because under some circumstances minimumCreditBorrowAToken will be compared to the incoming cash (instead of the end credit value).

Proof of Concept

minimumCreditBorrowAToken is used to ensure "the minimum credit value of loans" as it is stated in a code comment here.

Let's use buyCreditMarket for the example.

We call it with exactAmountIn == true and params.creditPositionId == RESERVED_ID. params.amount in this case will represent the amount of cash that will be is sent to the borrower minus the fees. The credit amount of the loan is going to be params.amount * PERCENT + ratePerTenor / PERCENT.

    function executeBuyCreditMarket(State storage state, BuyCreditMarketParams memory params)
        external
        returns (uint256 cashAmountIn)
    {
        ...
        if (params.exactAmountIn) {
            cashAmountIn = params.amount;
            (creditAmountOut, fees) = state.getCreditAmountOut({
                cashAmountIn: cashAmountIn,
                maxCashAmountIn: params.creditPositionId == RESERVED_ID
@>                  ? cashAmountIn
                    : Math.mulDivUp(creditPosition.credit, PERCENT, PERCENT + ratePerTenor),
                maxCredit: params.creditPositionId == RESERVED_ID
@>                  ? Math.mulDivDown(cashAmountIn, PERCENT + ratePerTenor, PERCENT)
                    : creditPosition.credit,
                ratePerTenor: ratePerTenor,
                tenor: tenor
            });
        } else {
            ...
        }

        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.data.borrowAToken.transferFrom(msg.sender, borrower, cashAmountIn - fees);
        state.data.borrowAToken.transferFrom(msg.sender, state.feeConfig.feeRecipient, fees);
    }

This means that under these circumstances params.amount could be below minimumCreditBorrowAToken but the credit amount of the loan could be above this threshold when we add the interest so the transaction should not revert.

minimumCreditBorrowAToken is used to ensure that the credit of the loans are going to be more than this value but this requirement should not be present for the cash that is coming in.

Tools Used

Manual Review

Recommended Mitigation Steps

Ensure that a revert occurs only when params.amount is representing credit that is below minimumCreditBorrowAToken or when the end loan credit amount is below this threshold too.

Assessed type

Invalid Validation

[C4-Staff]

CloudEllie marked the issue as duplicate of #224

[c4-judge]

hansfriese marked the issue as satisfactory