validateSellCreditMarket and validateBuyCreditMarket incorrectly assumes params.amount is credit when validating against minimumCreditBorrowAToken

[c4-bot-6]

Lines of code

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

Vulnerability details

Impact

When users calls sellCreditMarket, users can provide exactAmountIn flag to determine wether params. amount is credit or cash. However, validateSellCreditMarket incorrectly assume params.amount is credit when checking against state.riskConfig.minimumCreditBorrowAToken.

Proof of Concept

Inside validateSellCreditMarket, there are two validations related to params.amount. first, checking if it is greater than creditPosition.credit when params.creditPositionId is provided, and second, checking if params.amount is lower than minimumCreditBorrowAToken.

https://github.com/code-423n4/2024-06-size/blob/main/src/libraries/actions/SellCreditMarket.sol#L93-L95

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

        // validate creditPositionId
        if (params.creditPositionId == RESERVED_ID) {
            tenor = params.tenor;

            // validate tenor
            if (tenor < state.riskConfig.minTenor || tenor > state.riskConfig.maxTenor) {
                revert Errors.TENOR_OUT_OF_RANGE(tenor, state.riskConfig.minTenor, state.riskConfig.maxTenor);
            }
        } else {
            CreditPosition storage creditPosition = state.getCreditPosition(params.creditPositionId);
            DebtPosition storage debtPosition = state.getDebtPositionByCreditPositionId(params.creditPositionId);
            if (msg.sender != creditPosition.lender) {
                revert Errors.BORROWER_IS_NOT_LENDER(msg.sender, creditPosition.lender);
            }
            if (!state.isCreditPositionTransferrable(params.creditPositionId)) {
                revert Errors.CREDIT_POSITION_NOT_TRANSFERRABLE(
                    params.creditPositionId,
                    state.getLoanStatus(params.creditPositionId),
                    state.collateralRatio(debtPosition.borrower)
                );
            }
            tenor = debtPosition.dueDate - block.timestamp; // positive since the credit position is transferrable, so the loan must be ACTIVE

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

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

        // ...
    }

When checking against creditPosition.credit, it is safe to assume that params.amount is credit, since cash also cannot be greater than creditPosition.credit.

However, when checking against minimumCreditBorrowAToken and the provided amount is cash, it is possible that the cash is lower than minimumCreditBorrowAToken, while the actual credit exceeds minimumCreditBorrowAToken.

This could cause providing valid params.amount of cash to the operation could revert unexpectedly.

This is also the case inside validateBuyCreditMarket.

Tools Used

Manual review

Recommended Mitigation Steps

Remove the minimumCreditBorrowAToken validation inside validateSellCreditMarket and validateBuyCreditMarket, since it will be checked inside executeSellCreditMarket and executeBuyCreditMarket when credit is created.

Assessed type

Invalid Validation

[aviggiano]

Fixed in SizeCredit/size-solidity#125

[C4-Staff]

CloudEllie marked the issue as duplicate of #224

[c4-judge]

hansfriese marked the issue as satisfactory