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

3 stars 1 forks source link

executeSellCreditMarket() when creditPositionId == RESERVED_ID , Incorrect calculation of fees #85

Closed c4-bot-3 closed 4 months ago

c4-bot-3 commented 4 months ago

Lines of code

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

Vulnerability details

Vulnerability details

when call executeSellCreditMarket() We can specify exactAmountIn=false and params.creditPositionId == RESERVED_ID This will generate new CreditPositions The code is as follows:

    function executeSellCreditMarket(State storage state, SellCreditMarketParams calldata params)
...
        if (params.exactAmountIn) {
...
        } else {
            cashAmountOut = params.amount;

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

@>      state.data.borrowAToken.transferFrom(params.lender, msg.sender, cashAmountOut);
@>      state.data.borrowAToken.transferFrom(params.lender, state.feeConfig.feeRecipient, fees);
...

    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) {

The above code, mainly calculates creditAmountIn and fees based on cashAmountOut = params.amount. After that, cashAmountOut is transferred to msg.sender, fees is transferred to feeRecipient , and creditAmountIn is recorded as debt.

The calculation formula is:

  1. creditAmountIn = cashAmountOut * (1 + ratePerTenor)/ ( 1 - swapFeePercent) ----------> (correct)
  2. fees = cashAmountOut * swapFeePercent ----------> (wrong)

The second formula is wrong Because: cashAmountOut is what the actual user receives, swapFeePercent should not be multiplied by this value. The normal formula should be: fees = (cashAmountOut + fees ) * swapFeePercent Derived as: => fees = cashAmountOut * swapFeePercent / (1 - swapFeePercent)

This ensures that: (cashAmountOut + fees) * (1 + ratePerTenor) = creditAmountIn

Impact

lender pays fees that are always less than the correct value

Recommended Mitigation

    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, swapFeePercent, (PERCENT - swapFeePercent));
        } else if (cashAmountOut < maxCashAmountOutFragmentation) {

Assessed type

Math

c4-judge commented 4 months ago

hansfriese marked the issue as unsatisfactory: Invalid

c4-judge commented 4 months ago

hansfriese marked the issue as not a duplicate