Lines of code

https://github.com/code-423n4/2024-06-size/blob/main/src/libraries/AccountingLibrary.sol#L249

Vulnerability details

Impact

Swap fees are incorrectly calculated when selling credit as a whole when exactAmountIn == false.

Bug Description

Let's first see the formula for selling credit as a whole when exactAmountIn == false.

https://docs.size.credit/technical-docs/contracts/3.3-market-orders#id-3.3.1.3-technical-specification

The formula is A = (V+f)*(1+r)/(1-kt), where A is credit, V is amount of cash swapped, f is fragmentationFee, r is APR, kt is swapFeePercentage.

When we sell the credit as a whole, there is no fragmentationFee, and the amount of credit A is fixed, so the total amount of cash swapped is V = A/(1+r) * (1-kt). This is equal to the maxCashAmountOut in the following code where it is calculated as Math.mulDivDown(creditPosition.credit, PERCENT - state.getSwapFeePercent(tenor), PERCENT + ratePerTenor).

Let's break down the formula: A/(1+r) represents the total cash flow corresponding to the credit. From this, (1-kt) percent goes to the borrower (which is the above V), and kt percent should be counted as swap fees. Therefore, the swap fee should be equal to A/(1+r) * kt.

However, the current implementation uses fees = Math.mulDivUp(cashAmountOut, swapFeePercent, PERCENT), which is V * kt = A/(1+r) * (1-kt) * kt. This is incorrect and results in the fees being smaller than expected.

The correct implementation should be fees = Math.mulDivUp(cashAmountOut, PERCENT, (1 - swapFeePercent)) - cashAmountOut;.

AccountingLibrary.sol

    function getCreditAmountIn(
        State storage state,
        uint256 cashAmountOut,
        uint256 maxCashAmountOut,
        uint256 maxCredit,
        uint256 ratePerTenor,
        uint256 tenor
    ) internal view returns (uint256 creditAmountIn, uint256 fees) {
        uint256 swapFeePercent = getSwapFeePercent(state, tenor);

        uint256 maxCashAmountOutFragmentation = 0;

        if (maxCashAmountOut >= state.feeConfig.fragmentationFee) {
            maxCashAmountOutFragmentation = maxCashAmountOut - state.feeConfig.fragmentationFee;
        }

        // slither-disable-next-line incorrect-equality
        if (cashAmountOut == maxCashAmountOut) {
            // no credit fractionalization

            creditAmountIn = maxCredit;
>           fees = Math.mulDivUp(cashAmountOut, swapFeePercent, PERCENT);
        } else if (cashAmountOut < maxCashAmountOutFragmentation) {
            // credit fractionalization

            creditAmountIn = Math.mulDivUp(
                cashAmountOut + state.feeConfig.fragmentationFee, PERCENT + ratePerTenor, PERCENT - swapFeePercent
            );
            fees = Math.mulDivUp(cashAmountOut, swapFeePercent, PERCENT) + state.feeConfig.fragmentationFee;
        } else {
            // for maxCashAmountOutFragmentation < amountOut < maxCashAmountOut we are in an inconsistent situation
            //   where charging the swap fee would require to sell a credit that exceeds the max possible credit

            revert Errors.NOT_ENOUGH_CASH(maxCashAmountOutFragmentation, cashAmountOut);
        }
    }

SellCreditMarket.sol

        if (params.exactAmountIn) {
            creditAmountIn = params.amount;

            (cashAmountOut, fees) = state.getCashAmountOut({
                creditAmountIn: creditAmountIn,
                maxCredit: params.creditPositionId == RESERVED_ID ? creditAmountIn : creditPosition.credit,
                ratePerTenor: ratePerTenor,
                tenor: tenor
            });
        } else {
            cashAmountOut = params.amount;

            (creditAmountIn, fees) = state.getCreditAmountIn({
                cashAmountOut: cashAmountOut,
>               maxCashAmountOut: params.creditPositionId == RESERVED_ID
>                   ? cashAmountOut
>                   : Math.mulDivDown(creditPosition.credit, PERCENT - state.getSwapFeePercent(tenor), PERCENT + ratePerTenor),
                maxCredit: params.creditPositionId == RESERVED_ID
                    ? Math.mulDivUp(cashAmountOut, PERCENT + ratePerTenor, PERCENT - state.getSwapFeePercent(tenor))
                    : creditPosition.credit,
                ratePerTenor: ratePerTenor,
                tenor: tenor
            });
        }

Proof of Concept

An example of selling credit: Future credit is 11000, APR is 10%, swapFee percentage is 1%.

The total cash flow would be 11000 / (1 + 10%) = 10000.
Borrower should receive 10000 * (1 - 1%) = 9900.
Swap fee should be 10000 - 9900 = 100.

But the current implementation calculates the swap fee to be 9990 * 1% = 99.9.

Tools Used

Manual review

Recommended Mitigation Steps

    function getCreditAmountIn(
        State storage state,
        uint256 cashAmountOut,
        uint256 maxCashAmountOut,
        uint256 maxCredit,
        uint256 ratePerTenor,
        uint256 tenor
    ) internal view returns (uint256 creditAmountIn, uint256 fees) {
        uint256 swapFeePercent = getSwapFeePercent(state, tenor);

        uint256 maxCashAmountOutFragmentation = 0;

        if (maxCashAmountOut >= state.feeConfig.fragmentationFee) {
            maxCashAmountOutFragmentation = maxCashAmountOut - state.feeConfig.fragmentationFee;
        }

        // slither-disable-next-line incorrect-equality
        if (cashAmountOut == maxCashAmountOut) {
            // no credit fractionalization

            creditAmountIn = maxCredit;
-           fees = Math.mulDivUp(cashAmountOut, swapFeePercent, PERCENT);
+           fees = Math.mulDivUp(cashAmountOut, PERCENT, (1 - swapFeePercent)) - cashAmountOut;
        } else if (cashAmountOut < maxCashAmountOutFragmentation) {
            // credit fractionalization

            creditAmountIn = Math.mulDivUp(
                cashAmountOut + state.feeConfig.fragmentationFee, PERCENT + ratePerTenor, PERCENT - swapFeePercent
            );
            fees = Math.mulDivUp(cashAmountOut, swapFeePercent, PERCENT) + state.feeConfig.fragmentationFee;
        } else {
            // for maxCashAmountOutFragmentation < amountOut < maxCashAmountOut we are in an inconsistent situation
            //   where charging the swap fee would require to sell a credit that exceeds the max possible credit

            revert Errors.NOT_ENOUGH_CASH(maxCashAmountOutFragmentation, cashAmountOut);
        }
    }

Assessed type

Math

code-423n4 / 2024-06-size-findings

Swap fees are incorrectly calculated when selling credit as a whole when `exactAmountIn == false`. #54