BuyCreditMarket charges extra swap fees as AccountingLibrary's `getCashAmountIn()` and `getCreditAmountOut()` use incorrect base amount

[c4-bot-2]

Lines of code

https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/AccountingLibrary.sol#L311-L332 https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/AccountingLibrary.sol#L274-L297

Vulnerability details

Both functions use Math.mulDivUp(credit, PERCENT, PERCENT + ratePerTenor) as a base for swap fee, while it's to be paid by borrower using their borrow amount as a base, which is only a part of credit present value, while the other part are fees that were transferred by lender on behalf of borrower, i.e. fees are also borrowed and have to be covered by the total credit.

This way instead of amount * feePercent it now is amount * (1 + feePercent) * feePercent = amount * feePercent + amount * feePercent ^ 2, where the last part is charged incorrectly being fee on fee.

Impact

Swap fees are calculated from the amount that already includes them, i.e. what is charged is a swap fee and an another swap fee off the first swap fee. This is a violation of the stated protocol logic leading to user's loss on each BuyCreditMarket operation without any additional prerequisites.

Since the base can be misstated rather substantially when tenor is large with swap fees being double charged and there are no additional prerequisites for the impact, placing the severity to be high.

Proof of Concept

Swap fees are paid by the caller/lender on behalf of the borrower (cash receiver), who pay the fee off the amount borrowed, which is cashAmountIn - fees, not cashAmountIn:

AccountingLibrary.sol#L311-L332

    function getCashAmountIn(
        State storage state,
        uint256 creditAmountOut,
        uint256 maxCredit,
        uint256 ratePerTenor,
        uint256 tenor
    ) internal view returns (uint256 cashAmountIn, uint256 fees) {
        if (creditAmountOut == maxCredit) {
            // no credit fractionalization

            cashAmountIn = Math.mulDivUp(maxCredit, PERCENT, PERCENT + ratePerTenor);
>>          fees = getSwapFee(state, cashAmountIn, tenor);
        } else if (creditAmountOut < maxCredit) {
            // credit fractionalization

            uint256 netCashAmountIn = Math.mulDivUp(creditAmountOut, PERCENT, PERCENT + ratePerTenor);
            cashAmountIn = netCashAmountIn + state.feeConfig.fragmentationFee;

>>          fees = getSwapFee(state, netCashAmountIn, tenor) + state.feeConfig.fragmentationFee;
        } else {
            revert Errors.NOT_ENOUGH_CREDIT(creditAmountOut, maxCredit);
        }

The same situation is with getCreditAmountOut():

AccountingLibrary.sol#L274-L297

    function getCreditAmountOut(
        State storage state,
        uint256 cashAmountIn,
        uint256 maxCashAmountIn,
        uint256 maxCredit,
        uint256 ratePerTenor,
        uint256 tenor
    ) internal view returns (uint256 creditAmountOut, uint256 fees) {
        if (cashAmountIn == maxCashAmountIn) {
            // no credit fractionalization

            creditAmountOut = maxCredit;
>>          fees = getSwapFee(state, cashAmountIn, tenor);
        } else if (cashAmountIn < maxCashAmountIn) {
            // credit fractionalization

            if (state.feeConfig.fragmentationFee > cashAmountIn) {
                revert Errors.NOT_ENOUGH_CASH(state.feeConfig.fragmentationFee, cashAmountIn);
            }

            uint256 netCashAmountIn = cashAmountIn - state.feeConfig.fragmentationFee;

            creditAmountOut = Math.mulDivDown(netCashAmountIn, PERCENT + ratePerTenor, PERCENT);
>>          fees = getSwapFee(state, netCashAmountIn, tenor) + state.feeConfig.fragmentationFee;

cashAmountIn paid by the lender is a total sum inclusive of the swap fees, paid by the borrower/cash receiver, and fragmentation fees (if any) paid by the lender/caller.

I.e. it is borrower who receives the cash and they pay the swap fee and for them the base is amount borrowed, cashAmountIn - fees:

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

        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 {
            creditAmountOut = params.amount;
>>          (cashAmountIn, fees) = state.getCashAmountIn({
                creditAmountOut: creditAmountOut,
                maxCredit: params.creditPositionId == RESERVED_ID ? creditAmountOut : creditPosition.credit,
                ratePerTenor: ratePerTenor,
                tenor: tenor
            });
        }

        ...

>>      state.data.borrowAToken.transferFrom(msg.sender, borrower, cashAmountIn - fees);
>>      state.data.borrowAToken.transferFrom(msg.sender, state.feeConfig.feeRecipient, fees);

Tools Used

Manual Review

Recommended Mitigation Steps

Consider calculating the net amount of the loan for the borrower, cashAmountIn / (1 + swapFeePercent), e.g. for getCashAmountIn():

AccountingLibrary.sol#L311-L332

    function getCashAmountIn(
        ...
    ) internal view returns (uint256 cashAmountIn, uint256 fees) {
+       uint256 swapFeePercent = getSwapFeePercent(state, tenor);
        if (creditAmountOut == maxCredit) {
            // no credit fractionalization

            cashAmountIn = Math.mulDivUp(maxCredit, PERCENT, PERCENT + ratePerTenor);
-           fees = getSwapFee(state, cashAmountIn, tenor);
+           fees = Math.mulDivUp(Math.mulDivUp(cashAmountIn, PERCENT, PERCENT + swapFeePercent), swapFeePercent, PERCENT);
        } else if (creditAmountOut < maxCredit) {
            // credit fractionalization

            uint256 netCashAmountIn = Math.mulDivUp(creditAmountOut, PERCENT, PERCENT + ratePerTenor);
            cashAmountIn = netCashAmountIn + state.feeConfig.fragmentationFee;

-           fees = getSwapFee(state, netCashAmountIn, tenor) + state.feeConfig.fragmentationFee;
+           fees = Math.mulDivUp(Math.mulDivUp(netCashAmountIn, PERCENT, PERCENT + swapFeePercent), swapFeePercent, PERCENT) + state.feeConfig.fragmentationFee;
        } else {
            revert Errors.NOT_ENOUGH_CREDIT(creditAmountOut, maxCredit);
        }

getCreditAmountOut() can be modified to use cashAmountIn / (1 + swapFeePercent) as the swap fee base in the same manner.

Assessed type

Math