Incorrect Fee Calculation in Credit Fractionalization Scenario

[howlbot-integration[bot]]

Lines of code

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

Vulnerability details

Description:

The getCashAmountOut function in the AccountingLibrary contains a logical error in its handling of fees during credit fractionalization scenarios. This error can lead to an underestimation of the cash amount returned to users, potentially causing financial losses or unexpected behavior in the protocol.

Code:

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

function getCashAmountOut(
    State storage state,
    uint256 creditAmountIn,
    uint256 maxCredit,
    uint256 ratePerTenor,
    uint256 tenor
) internal view returns (uint256 cashAmountOut, uint256 fees) {
    uint256 maxCashAmountOut = Math.mulDivDown(creditAmountIn, PERCENT, PERCENT + ratePerTenor);

    if (creditAmountIn == maxCredit) {
        // no credit fractionalization
        fees = getSwapFee(state, maxCashAmountOut, tenor);
        if (fees > maxCashAmountOut) {
            revert Errors.NOT_ENOUGH_CASH(maxCashAmountOut, fees);
        }
        cashAmountOut = maxCashAmountOut - fees;
    } else if (creditAmountIn < maxCredit) {
        // credit fractionalization
        fees = getSwapFee(state, maxCashAmountOut, tenor) + state.feeConfig.fragmentationFee;
        if (fees > maxCashAmountOut) {
            revert Errors.NOT_ENOUGH_CASH(maxCashAmountOut, fees);
        }
        cashAmountOut = maxCashAmountOut - fees;
    } else {
        revert Errors.NOT_ENOUGH_CREDIT(creditAmountIn, maxCredit);
    }
}

Details:

In the credit fractionalization case (when creditAmountIn < maxCredit), the function calculates fees and cash output incorrectly:

1. It calculates maxCashAmountOut based on the full creditAmountIn, without considering the fragmentation fee.
2. It then calculates the swap fee based on this maxCashAmountOut.
3. The fragmentation fee is added to the swap fee to get the total fees.
4. Finally, it subtracts the total fees from maxCashAmountOut to get cashAmountOut.

This approach leads to an overestimation of the swap fee and an underestimation of the cashAmountOut. The fragmentation fee is effectively double-counted in the calculation, reducing the cash output more than it should.

Impact:

This bug can lead to users receiving less cash than they should when performing credit fractionalization operations. The impact is proportional to the size of the fragmentation fee and the amount of credit being fractionalized. In scenarios with large credit amounts or high fragmentation fees, the financial impact could be significant.

Proof of Concept:

Consider a scenario with the following parameters:

• creditAmountIn = 1000
• ratePerTenor = 5%
• Swap fee = 1%
• Fragmentation fee = 10

Correct calculation:

1. maxCashAmountOut = 1000 * 100 / 105 ≈ 952.38
2. netCashAmountOut = 952.38 - 10 = 942.38
3. Swap fee = 942.38 * 0.01 ≈ 9.42
4. Total fees = 9.42 + 10 = 19.42
5. cashAmountOut = 942.38 - 9.42 = 932.96

Current implementation:

1. maxCashAmountOut = 1000 * 100 / 105 ≈ 952.38
2. Swap fee = 952.38 * 0.01 ≈ 9.52
3. Total fees = 9.52 + 10 = 19.52
4. cashAmountOut = 952.38 - 19.52 = 932.86

The difference of 0.10 represents the error in the calculation.

Recommended Fix:

Modify the credit fractionalization case in the getCashAmountOut function to correctly account for the fragmentation fee:

} else if (creditAmountIn < maxCredit) {
    // credit fractionalization
    if (state.feeConfig.fragmentationFee > maxCashAmountOut) {
        revert Errors.NOT_ENOUGH_CASH(maxCashAmountOut, state.feeConfig.fragmentationFee);
    }
    uint256 netCashAmountOut = maxCashAmountOut - state.feeConfig.fragmentationFee;
    uint256 swapFee = getSwapFee(state, netCashAmountOut, tenor);
    fees = swapFee + state.feeConfig.fragmentationFee;
    if (fees > maxCashAmountOut) {
        revert Errors.NOT_ENOUGH_CASH(maxCashAmountOut, fees);
    }
    cashAmountOut = netCashAmountOut - swapFee;
}

This modification ensures that the fragmentation fee is properly accounted for in the calculations, providing a more accurate and fair cashAmountOut value in the case of credit fractionalization.

Assessed type

Math

[c4-judge]

hansfriese marked the issue as unsatisfactory: Invalid

[c4-judge]

hansfriese changed the severity to 2 (Med Risk)