Market Fees Calculation Issue

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/Size.sol#L188-L195 https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/actions/SellCreditMarket.sol#L127-L204

Vulnerability details

Summary

When users buy or sell a credit position, fees are applied based on whether they provide the amount IN or the amount OUT. Depending on which amount is specified, different functions are triggered to calculate these fees and the corresponding amounts.

In some cases, the expected reciprocity between input and output amounts and associated fees is not maintained. For example, when Y amount IN results in X amount OUT with W fees, the reverse scenario where X amount IN does not result in Y amount OUT and W fees introduces discrepancies and inequalities among users.

Description

The issue arises in the buyCreditMarket and sellCreditMarket functions within size.sol. For the sake of clarity, let's focus on sellCreditMarket.

The problem manifests when users attempt to purchase a limit offer using the special ID RESERVED_ID. The critical function involved is executeSellCreditMarket:

https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/Size.sol#L188-L195

    function sellCreditMarket(SellCreditMarketParams memory params) external payable override(ISize) whenNotPaused {
        state.validateSellCreditMarket(params);
        uint256 amount = state.executeSellCreditMarket(params);
        if (params.creditPositionId == RESERVED_ID) {
            state.validateUserIsNotBelowOpeningLimitBorrowCR(msg.sender);
        }
        state.validateVariablePoolHasEnoughLiquidity(amount);
    }

https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/actions/SellCreditMarket.sol#L127-L204

    function executeSellCreditMarket(State storage state, SellCreditMarketParams calldata params)
        external
        returns (uint256 cashAmountOut)
    {
        emit Events.SellCreditMarket(
            params.lender, params.creditPositionId, params.tenor, params.amount, params.tenor, params.exactAmountIn
        );

        // slither-disable-next-line uninitialized-local
        CreditPosition memory creditPosition;
        uint256 tenor;
        if (params.creditPositionId == RESERVED_ID) {
            tenor = params.tenor;
        } else {
            DebtPosition storage debtPosition = state.getDebtPositionByCreditPositionId(params.creditPositionId);
            creditPosition = state.getCreditPosition(params.creditPositionId);

            tenor = debtPosition.dueDate - block.timestamp;
        }

        uint256 ratePerTenor = state.data.users[params.lender].loanOffer.getRatePerTenor(
            VariablePoolBorrowRateParams({
                variablePoolBorrowRate: state.oracle.variablePoolBorrowRate,
                variablePoolBorrowRateUpdatedAt: state.oracle.variablePoolBorrowRateUpdatedAt,
                variablePoolBorrowRateStaleRateInterval: state.oracle.variablePoolBorrowRateStaleRateInterval
            }),
            tenor
        );

        uint256 creditAmountIn;
        uint256 fees;

        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
            });
        }

        if (params.creditPositionId == RESERVED_ID) {
            // slither-disable-next-line unused-return
            state.createDebtAndCreditPositions({
                lender: msg.sender,
                borrower: msg.sender,
                futureValue: creditAmountIn,
                dueDate: block.timestamp + tenor
            });
        }

        state.createCreditPosition({
            exitCreditPositionId: params.creditPositionId == RESERVED_ID
                ? state.data.nextCreditPositionId - 1
                : params.creditPositionId,
            lender: params.lender,
            credit: creditAmountIn
        });

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

In these functions, the calculation of creditAmountIn, cashAmountOut, and fees differs depending on whether params.amount is considered as the input amount (params.exactAmountIn == TRUE) or the output amount (params.exactAmountIn == FALSE).This differentiation should maintain consistency in both directions to ensure fairness and equality among users.

        uint256 creditAmountIn;
        uint256 fees;

        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
            });
        }

Assertion: When a specific amount X is used as creditAmountIn resulting in cashAmountOut and fees with params.exactAmountIn == TRUE, the same creditAmountIn and fees should be obtained when params.exactAmountIn == FALSE, and the cashAmountOut equal the previous cashAmountOut.

POC

To validate the consistency between the getCashAmountOut and getCreditAmountIn functions, a series of tests were conducted using Echidna. These tests were designed to chain the output of one function as input into the other, ensuring that the initial input value is preserved as the output of the second function.

Test Parameters

Each function was parameterized with specific values to ensure comprehensive testing:

    creditPositionId = creditPositionId % 2 == 0 ? RESERVED_ID : 0;
    tenor = between(tenor, 1 days, 5 * 365 days);
    amount = between(amount, 50e6, MAX_AMOUNT_USDC);
    credit = between(credit, 50e6, MAX_AMOUNT_USDC);
    ratePerTenor = between(ratePerTenor, 0.001e18, 0.1e18);

Test Cases

Four specific test cases were implemented in /test/invariants/TargetFunctions.sol to cover different scenarios with specific input bounds:

    function inputExactOutputSell1Fees(
        uint256 creditPositionId,
        uint256 tenor,
        uint256 amount,
        uint256 credit,
        uint256 ratePerTenor
    ) public {

        uint256 cashAmountOut;
        uint256 creditAmountIn;
        uint256 creditAmountIn2;
        uint256 fees1;
        uint256 fees2;

        creditPositionId = creditPositionId % 2 == 0 ? RESERVED_ID : 0;
        tenor = between(tenor, 1 days, 5 * 365 days);
        amount = between(amount, 50e6, MAX_AMOUNT_USDC);
        credit = between(credit, 50e6, MAX_AMOUNT_USDC);
        ratePerTenor = between(ratePerTenor, 0.001e18, 0.1e18);

            creditAmountIn = amount;
            (cashAmountOut, fees1) = getCashAmountOut({
                creditAmountIn: creditAmountIn,
                maxCredit: creditPositionId == RESERVED_ID ? creditAmountIn : credit,
                ratePerTenor: ratePerTenor,
                tenor: tenor
            });

            (creditAmountIn2, fees2) = getCreditAmountIn({
                cashAmountOut: cashAmountOut,
                maxCashAmountOut: creditPositionId == RESERVED_ID
                    ? cashAmountOut
                    : Math.mulDivDown(credit, PERCENT - getSwapFeePercent(tenor), PERCENT + ratePerTenor),
                maxCredit: creditPositionId == RESERVED_ID
                    ? Math.mulDivUp(cashAmountOut, PERCENT + ratePerTenor, PERCENT - getSwapFeePercent(tenor))
                    : credit,
                ratePerTenor: ratePerTenor,
                tenor: tenor
            });

            //eq(amount, creditAmountIn2, "input not exact to output1");
            eq(fees1, fees2, "fees should be equal 1");
    }

    function inputExactOutputSell1Amount(
        uint256 creditPositionId,
        uint256 tenor,
        uint256 amount,
        uint256 credit,
        uint256 ratePerTenor
    ) public {

        uint256 cashAmountOut;
        uint256 creditAmountIn;
        uint256 creditAmountIn2;
        uint256 fees1;
        uint256 fees2;

        creditPositionId = creditPositionId % 2 == 0 ? RESERVED_ID : 0;
        tenor = between(tenor, 1 days, 5 * 365 days);
        amount = between(amount, 50e6, MAX_AMOUNT_USDC);
        credit = between(credit, 50e6, MAX_AMOUNT_USDC);
        ratePerTenor = between(ratePerTenor, 0.001e18, 0.1e18);

            creditAmountIn = amount;
            (cashAmountOut, fees1) = getCashAmountOut({
                creditAmountIn: creditAmountIn,
                maxCredit: creditPositionId == RESERVED_ID ? creditAmountIn : credit,
                ratePerTenor: ratePerTenor,
                tenor: tenor
            });

            (creditAmountIn2, fees2) = getCreditAmountIn({
                cashAmountOut: cashAmountOut,
                maxCashAmountOut: creditPositionId == RESERVED_ID
                    ? cashAmountOut
                    : Math.mulDivDown(credit, PERCENT - getSwapFeePercent(tenor), PERCENT + ratePerTenor),
                maxCredit: creditPositionId == RESERVED_ID
                    ? Math.mulDivUp(cashAmountOut, PERCENT + ratePerTenor, PERCENT - getSwapFeePercent(tenor))
                    : credit,
                ratePerTenor: ratePerTenor,
                tenor: tenor
            });

            eq(creditAmountIn, creditAmountIn2, "input not exact to output1");
            //eq(fees1, fees2, "fees should be equal 1");
    }

    function inputExactOutputSell2Fees(
        uint256 creditPositionId,
        uint256 tenor,
        uint256 amount,
        uint256 credit,
        uint256 ratePerTenor
    ) public {

        uint256 cashAmountOut;
        uint256 cashAmountOut2;
        uint256 creditAmountIn;
        uint256 fees1;
        uint256 fees2;

        creditPositionId = creditPositionId % 2 == 0 ? RESERVED_ID : 0;
        tenor = between(tenor, 1 days, 5 * 365 days);
        amount = between(amount, 50e6, MAX_AMOUNT_USDC);
        credit = between(credit, 50e6, MAX_AMOUNT_USDC);
        ratePerTenor = between(ratePerTenor, 0.001e18, 0.1e18);

        //console.log("amount", amount);

            cashAmountOut = amount;
            (creditAmountIn, fees2) = getCreditAmountIn({
                cashAmountOut: cashAmountOut,
                maxCashAmountOut: creditPositionId == RESERVED_ID
                    ? cashAmountOut
                    : Math.mulDivDown(credit, PERCENT - getSwapFeePercent(tenor), PERCENT + ratePerTenor),
                maxCredit: creditPositionId == RESERVED_ID
                    ? Math.mulDivUp(cashAmountOut, PERCENT + ratePerTenor, PERCENT - getSwapFeePercent(tenor))
                    : credit,
                ratePerTenor: ratePerTenor,
                tenor: tenor
            });

            (cashAmountOut2, fees1) = getCashAmountOut({
                creditAmountIn: creditAmountIn,
                maxCredit: creditPositionId == RESERVED_ID ? creditAmountIn : credit,
                ratePerTenor: ratePerTenor,
                tenor: tenor
            });

            //eq(amount, cashAmountOut2, "input not exact to output2");
            eq(fees1, fees2, "fees should be equal 2");
            /* require(amount == cashAmountOut2, "input not exact to output2");
            require(fees1 == fees2, "fees should be equal 2"); */

    }

    function inputExactOutputSell2Amount(
        uint256 creditPositionId,
        uint256 tenor,
        uint256 amount,
        uint256 credit,
        uint256 ratePerTenor
    ) public {

        uint256 cashAmountOut;
        uint256 cashAmountOut2;
        uint256 creditAmountIn;
        uint256 fees1;
        uint256 fees2;

        creditPositionId = creditPositionId % 2 == 0 ? RESERVED_ID : 0;
        tenor = between(tenor, 1 days, 5 * 365 days);
        amount = between(amount, 50e6, MAX_AMOUNT_USDC);
        credit = between(credit, 50e6, MAX_AMOUNT_USDC);
        ratePerTenor = between(ratePerTenor, 0.001e18, 0.1e18);

        //console.log("amount", amount);

            cashAmountOut = amount;
            (creditAmountIn, fees2) = getCreditAmountIn({
                cashAmountOut: cashAmountOut,
                maxCashAmountOut: creditPositionId == RESERVED_ID
                    ? cashAmountOut
                    : Math.mulDivDown(credit, PERCENT - getSwapFeePercent(tenor), PERCENT + ratePerTenor),
                maxCredit: creditPositionId == RESERVED_ID
                    ? Math.mulDivUp(cashAmountOut, PERCENT + ratePerTenor, PERCENT - getSwapFeePercent(tenor))
                    : credit,
                ratePerTenor: ratePerTenor,
                tenor: tenor
            });

            (cashAmountOut2, fees1) = getCashAmountOut({
                creditAmountIn: creditAmountIn,
                maxCredit: creditPositionId == RESERVED_ID ? creditAmountIn : credit,
                ratePerTenor: ratePerTenor,
                tenor: tenor
            });

            eq(cashAmountOut, cashAmountOut2, "input not exact to output2");
            //eq(fees1, fees2, "fees should be equal 2");
            /* require(amount == cashAmountOut2, "input not exact to output2");
            require(fees1 == fees2, "fees should be equal 2"); */

    }

    function getCreditAmountIn(
        uint256 cashAmountOut,
        uint256 maxCashAmountOut,
        uint256 maxCredit,
        uint256 ratePerTenor,
        uint256 tenor
    ) internal view returns (uint256 creditAmountIn, uint256 fees) {
        uint256 fragmentationFee = 5e6;
        uint256 swapFeePercent = getSwapFeePercent( tenor);

        uint256 maxCashAmountOutFragmentation = 0;

        if (maxCashAmountOut >= fragmentationFee) {
            maxCashAmountOutFragmentation = maxCashAmountOut - 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 + fragmentationFee, PERCENT + ratePerTenor, PERCENT - swapFeePercent
            );
            fees = Math.mulDivUp(cashAmountOut, swapFeePercent, PERCENT) + 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);
        }
    }

    function getCreditAmountOut(
        uint256 cashAmountIn,
        uint256 maxCashAmountIn,
        uint256 maxCredit,
        uint256 ratePerTenor,
        uint256 tenor
    ) internal view returns (uint256 creditAmountOut, uint256 fees) {
        uint256 fragmentationFee = 5e6;
        if (cashAmountIn == maxCashAmountIn) {
            // no credit fractionalization

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

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

            uint256 netCashAmountIn = cashAmountIn - fragmentationFee;

            creditAmountOut = Math.mulDivDown(netCashAmountIn, PERCENT + ratePerTenor, PERCENT);
            fees = getSwapFee( netCashAmountIn, tenor) + fragmentationFee;
        } else {
            revert Errors.NOT_ENOUGH_CREDIT(maxCashAmountIn, cashAmountIn);
        }
    }

    function getCashAmountIn(
        uint256 creditAmountOut,
        uint256 maxCredit,
        uint256 ratePerTenor,
        uint256 tenor
    ) internal view returns (uint256 cashAmountIn, uint256 fees) {
        uint256 fragmentationFee = 5e6;
        if (creditAmountOut == maxCredit) {
            // no credit fractionalization

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

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

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

        if (fees > cashAmountIn) {
            revert Errors.NOT_ENOUGH_CASH(cashAmountIn, fees);
        }
    }

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

        if (creditAmountIn == maxCredit) {
            // no credit fractionalization

            fees = getSwapFee( maxCashAmountOut, tenor);

            if (fees > maxCashAmountOut) {
                revert Errors.NOT_ENOUGH_CASH(maxCashAmountOut, fees);
            }

            cashAmountOut = maxCashAmountOut - fees;
        } else if (creditAmountIn < maxCredit) {
            // credit fractionalization

            fees = getSwapFee(maxCashAmountOut, tenor) + fragmentationFee;

            if (fees > maxCashAmountOut) {
                revert Errors.NOT_ENOUGH_CASH(maxCashAmountOut, fees);
            }

            cashAmountOut = maxCashAmountOut - fees;
        } else {
            revert Errors.NOT_ENOUGH_CREDIT(creditAmountIn, maxCredit);
        }
    }

    function getSwapFee(uint256 cash, uint256 tenor) internal view returns (uint256) {
        return Math.mulDivUp(cash, getSwapFeePercent(tenor), PERCENT);
    }

    function getSwapFeePercent( uint256 tenor) internal view returns (uint256) {
        return Math.mulDivUp(0.005e18, tenor, 365 days);
    }

Echidna Output

The output from Echidna highlighted several failures where the assertions did not hold true, indicating discrepancies between expected and actual results in fees and amounts. This included cases where:

• Fees were expected to be equal but differed.
• Amounts were expected to match but showed variance.
• Both fees and amounts showed inconsistencies across different test scenarios.

inputExactOutputSell2Amount(uint256,uint256,uint256,uint256,uint256): failed!💥  
  Call sequence:
    CryticTester.inputExactOutputSell2Amount(7579179664,111281,805790477410442332139,50,0)

Traces: 
emit Log(«input not exact to output2») 

selfLiquidate(uint256): passing
setUserConfiguration(uint256,bool): passing
inputExactOutputSell1Fees(uint256,uint256,uint256,uint256,uint256): failed!💥  
  Call sequence:
    CryticTester.inputExactOutputSell1Fees(0,96,0,0,0)

Traces: 
emit Log(«fees should be equal 1») 

repay(uint256): passing
claim(uint256): passing
deposit(address,uint256): passing
sellCreditMarket(address,uint256,uint256,uint256,bool): passing
updateConfig(uint256,uint256): passing
compensate(uint256,uint256,uint256): passing
setPrice(uint256): passing
buyCreditLimit(uint256,uint256): passing
setLiquidityIndex(uint256,uint256): passing
liquidateWithReplacement(uint256,uint256,address): passing
liquidate(uint256,uint256): passing
buyCreditMarket(address,uint256,uint256,uint256,bool): passing
sellCreditLimit(uint256): passing
inputExactOutputSell2Fees(uint256,uint256,uint256,uint256,uint256): failed!💥  
  Call sequence:
    CryticTester.inputExactOutputSell2Fees(559309199367000419123,0,0,5070622,0)

Traces: 
emit Log(«fees should be equal 2») 

inputExactOutputSell1Amount(uint256,uint256,uint256,uint256,uint256): failed!💥  
  Call sequence:
    CryticTester.inputExactOutputSell1Amount(1674068349690332115854207766,39537462395223871,236739009463665944922313,0,0)

Traces: 
emit Log(«input not exact to output1») 

withdraw(address,uint256): passing
AssertionFailed(..): passing

Local reproduction wih foundry

To further validate these findings locally, tests were reproduced using Foundry. Create a file in test/libraries/ FeesTest.t.sol and copy paste this in it:

pragma solidity 0.8.23;

import {Test} from "forge-std/Test.sol";

import {AssertsHelper} from "@test/helpers/AssertsHelper.sol";

import {CryticAsserts} from "@chimera/CryticAsserts.sol";

enum LoanStatus {
    // When the loan is created, it is in ACTIVE status
    ACTIVE,
    // When tenor is reached, it is in OVERDUE status and subject to liquidation
    OVERDUE,
    // When the loan is repaid either by the borrower or by the liquidator, it is in REPAID status
    REPAID
}

contract FeesTest is Test {

    event Log(string);
    event LogUint(uint256);

    error NOT_ENOUGH_CASH(uint256 cash, uint256 required);
    error NOT_ENOUGH_CREDIT(uint256 credit, uint256 required);

    uint256 RESERVED_ID = type(uint256).max;
    uint256 PERCENT = 1e18;
    uint256 internal MAX_AMOUNT_USDC = 100_000_000e6;

    function getCreditAmountIn(
        uint256 cashAmountOut,
        uint256 maxCashAmountOut,
        uint256 maxCredit,
        uint256 ratePerTenor,
        uint256 tenor
    ) internal view returns (uint256 creditAmountIn, uint256 fees) {
        uint256 fragmentationFee = 5e6;
        uint256 swapFeePercent = getSwapFeePercent( tenor);

        uint256 maxCashAmountOutFragmentation = 0;

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

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

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

            creditAmountIn = mulDivUp(
                cashAmountOut + fragmentationFee, PERCENT + ratePerTenor, PERCENT - swapFeePercent
            );
            fees = mulDivUp(cashAmountOut, swapFeePercent, PERCENT) + 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 NOT_ENOUGH_CASH(maxCashAmountOutFragmentation, cashAmountOut);
        }
    }

    function getCreditAmountOut(
        uint256 cashAmountIn,
        uint256 maxCashAmountIn,
        uint256 maxCredit,
        uint256 ratePerTenor,
        uint256 tenor
    ) internal view returns (uint256 creditAmountOut, uint256 fees) {
        uint256 fragmentationFee = 5e6;
        if (cashAmountIn == maxCashAmountIn) {
            // no credit fractionalization

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

            if (fragmentationFee > cashAmountIn) {
                revert NOT_ENOUGH_CASH(fragmentationFee, cashAmountIn);
            }

            uint256 netCashAmountIn = cashAmountIn - fragmentationFee;

            creditAmountOut = mulDivDown(netCashAmountIn, PERCENT + ratePerTenor, PERCENT);
            fees = getSwapFee( netCashAmountIn, tenor) + fragmentationFee;
        } else {
            revert NOT_ENOUGH_CREDIT(maxCashAmountIn, cashAmountIn);
        }
    }

    function getCashAmountIn(
        uint256 creditAmountOut,
        uint256 maxCredit,
        uint256 ratePerTenor,
        uint256 tenor
    ) internal view returns (uint256 cashAmountIn, uint256 fees) {
        uint256 fragmentationFee = 5e6;
        if (creditAmountOut == maxCredit) {
            // no credit fractionalization

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

            uint256 netCashAmountIn = mulDivUp(creditAmountOut, PERCENT, PERCENT + ratePerTenor);
            cashAmountIn = netCashAmountIn + fragmentationFee;

            fees = getSwapFee(netCashAmountIn, tenor) + fragmentationFee;
        } else {
            revert NOT_ENOUGH_CREDIT(creditAmountOut, maxCredit);
        }

        if (fees > cashAmountIn) {
            revert NOT_ENOUGH_CASH(cashAmountIn, fees);
        }
    }

    function getCashAmountOut(
        uint256 creditAmountIn,
        uint256 maxCredit,
        uint256 ratePerTenor,
        uint256 tenor
    ) internal view returns (uint256 cashAmountOut, uint256 fees) {
        uint256 fragmentationFee = 5e6;
        uint256 maxCashAmountOut = mulDivDown(creditAmountIn, PERCENT, PERCENT + ratePerTenor);

        if (creditAmountIn == maxCredit) {
            // no credit fractionalization

            fees = getSwapFee( maxCashAmountOut, tenor);

            if (fees > maxCashAmountOut) {
                revert NOT_ENOUGH_CASH(maxCashAmountOut, fees);
            }

            cashAmountOut = maxCashAmountOut - fees;
        } else if (creditAmountIn < maxCredit) {
            // credit fractionalization

            fees = getSwapFee(maxCashAmountOut, tenor) + fragmentationFee;

            if (fees > maxCashAmountOut) {
                revert NOT_ENOUGH_CASH(maxCashAmountOut, fees);
            }

            cashAmountOut = maxCashAmountOut - fees;
        } else {
            revert NOT_ENOUGH_CREDIT(creditAmountIn, maxCredit);
        }
    }

    function getSwapFee(uint256 cash, uint256 tenor) internal view returns (uint256) {
        return mulDivUp(cash, getSwapFeePercent(tenor), PERCENT);
    }

    function getSwapFeePercent( uint256 tenor) internal view returns (uint256) {
        return mulDivUp(0.005e18, tenor, 365 days);
    }

    ////////////////////////
    // replace fn from lib//
    ////////////////////////
    function mulDivDown(uint256 x, uint256 y, uint256 z) internal pure returns (uint256) {
        return mulDiv(x, y, z);
    }

    function mulDiv(uint256 x, uint256 y, uint256 d) internal pure returns (uint256 z) {
        /// @solidity memory-safe-assembly
        assembly {
            // Equivalent to require(d != 0 && (y == 0 || x <= type(uint256).max / y))
            if iszero(mul(d, iszero(mul(y, gt(x, div(not(0), y)))))) {
                mstore(0x00, 0xad251c27) // `MulDivFailed()`.
                revert(0x1c, 0x04)
            }
            z := div(mul(x, y), d)
        }
    }

    function mulDivUp(uint256 x, uint256 y, uint256 z) internal pure returns (uint256) {
        return mulDivUp_(x, y, z);
    }

    function mulDivUp_(uint256 x, uint256 y, uint256 d) internal pure returns (uint256 z) {
        /// @solidity memory-safe-assembly
        assembly {
            // Equivalent to require(d != 0 && (y == 0 || x <= type(uint256).max / y))
            if iszero(mul(d, iszero(mul(y, gt(x, div(not(0), y)))))) {
                mstore(0x00, 0xad251c27) // `MulDivFailed()`.
                revert(0x1c, 0x04)
            }
            z := add(iszero(iszero(mod(mul(x, y), d))), div(mul(x, y), d))
        }
    }

    function between(uint256 value, uint256 low, uint256 high) internal virtual returns (uint256) {
        if (value < low || value > high) {
            uint256 ans = low + (value % (high - low + 1));
            return ans;
        }
        return value;
    }

    function eq(uint256 a, uint256 b, string memory reason) internal virtual {
        if (!(a == b)) {
            emit Log(reason);
            assert(false);
        }
    }

    function inputExactOutputSell1(
        uint256 creditPositionId,
        uint256 tenor,
        uint256 amount,
        uint256 credit,
        uint256 ratePerTenor
    ) public {

        uint256 cashAmountOut;
        uint256 creditAmountIn;
        uint256 creditAmountIn2;
        uint256 fees1;
        uint256 fees2;

        creditPositionId = creditPositionId % 2 == 0 ? RESERVED_ID : 0;
        tenor = between(tenor, 1 days, 5 * 365 days);
        amount = between(amount, 50e6, MAX_AMOUNT_USDC);
        credit = between(credit, 50e6, MAX_AMOUNT_USDC);
        ratePerTenor = between(ratePerTenor, 0.001e18, 0.1e18);

        creditAmountIn = amount;
        (cashAmountOut, fees1) = getCashAmountOut({
            creditAmountIn: creditAmountIn,
            maxCredit: creditPositionId == RESERVED_ID ? creditAmountIn : credit,
            ratePerTenor: ratePerTenor,
            tenor: tenor
        });

        emit LogUint(cashAmountOut);

        (creditAmountIn2, fees2) = getCreditAmountIn({
            cashAmountOut: cashAmountOut,
            maxCashAmountOut: creditPositionId == RESERVED_ID
                ? cashAmountOut
                : mulDivDown(credit, PERCENT - getSwapFeePercent(tenor), PERCENT + ratePerTenor),
            maxCredit: creditPositionId == RESERVED_ID
                ? mulDivUp(cashAmountOut, PERCENT + ratePerTenor, PERCENT - getSwapFeePercent(tenor))
                : credit,
            ratePerTenor: ratePerTenor,
            tenor: tenor
        });

        emit Log("fees1");
        emit LogUint(fees1);
        emit Log("fees2");
        emit LogUint(fees2);

        emit Log("creditAmountIn");
        emit LogUint(creditAmountIn);
        emit Log("creditAmountIn2");
        emit LogUint(creditAmountIn2);

        require(creditAmountIn == creditAmountIn2, "input not exact to output1");
        require(fees1 == fees2, "fees should be equal 1");
    }

    function check_inputExactOutputSell1(
        //uint256 creditPositionId,
        uint256 tenor,
        uint256 amount,
        //uint256 credit,
        uint256 ratePerTenor
    ) public {

        uint256 cashAmountOut;
        uint256 creditAmountIn;
        uint256 creditAmountIn2;
        uint256 fees1;
        uint256 fees2;

        vm.assume(tenor > 1 days ); 
        vm.assume(tenor < 5 * 365 days);

        vm.assume(amount > 50e6);
        vm.assume(amount < MAX_AMOUNT_USDC);

        vm.assume(ratePerTenor > 0.001e18);
        vm.assume(ratePerTenor < 0.25e18);

        creditAmountIn = amount;
        (cashAmountOut, fees1) = getCashAmountOut({
            creditAmountIn: creditAmountIn,
            maxCredit: creditAmountIn,
            ratePerTenor: ratePerTenor,
            tenor: tenor
        });

        (creditAmountIn2, fees2) = getCreditAmountIn({
            cashAmountOut: cashAmountOut,
            maxCashAmountOut: cashAmountOut,
            maxCredit: mulDivUp(cashAmountOut, PERCENT + ratePerTenor, PERCENT - getSwapFeePercent(tenor)),
            ratePerTenor: ratePerTenor,
            tenor: tenor
        });

        assertEq(creditAmountIn, creditAmountIn2);
        assertEq(fees1, fees2);
    }

    function inputExactOutputSell2(
        uint256 creditPositionId,
        uint256 tenor,
        uint256 amount,
        uint256 credit,
        uint256 ratePerTenor
    ) public {

        uint256 cashAmountOut;
        uint256 cashAmountOut2;
        uint256 creditAmountIn;
        uint256 fees1;
        uint256 fees2;

        creditPositionId = creditPositionId % 2 == 0 ? RESERVED_ID : 0;
        tenor = between(tenor, 1 days, 5 * 365 days);
        amount = between(amount, 50e6, MAX_AMOUNT_USDC);
        credit = between(credit, 50e6, MAX_AMOUNT_USDC);
        ratePerTenor = between(ratePerTenor, 0.001e18, 0.1e18);

        emit Log("creditPositionId");
        emit LogUint(creditPositionId);

        emit Log("tenor");
        emit LogUint(tenor);

        emit Log("amount");
        emit LogUint(amount);

        emit Log("credit");
        emit LogUint(credit);

        emit Log("ratePerTenor");
        emit LogUint(ratePerTenor);

        cashAmountOut = amount;
        (creditAmountIn, fees2) = getCreditAmountIn({
            cashAmountOut: cashAmountOut,
            maxCashAmountOut: creditPositionId == RESERVED_ID
                ? cashAmountOut
                : mulDivDown(credit, PERCENT - getSwapFeePercent(tenor), PERCENT + ratePerTenor),
            maxCredit: creditPositionId == RESERVED_ID
                ? mulDivUp(cashAmountOut, PERCENT + ratePerTenor, PERCENT - getSwapFeePercent(tenor))
                : credit,
            ratePerTenor: ratePerTenor,
            tenor: tenor
        });

        emit Log("Middle creditAmountIn");
        emit LogUint(creditAmountIn/1e6);

        (cashAmountOut2, fees1) = getCashAmountOut({
            creditAmountIn: creditAmountIn,
            maxCredit: creditPositionId == RESERVED_ID ? creditAmountIn : credit,
            ratePerTenor: ratePerTenor,
            tenor: tenor
        });

        emit Log("fees1");
        emit LogUint(fees1);
        emit Log("fees2");
        emit LogUint(fees2);

        emit Log("cashAmountOut");
        emit LogUint(cashAmountOut);
        emit Log("cashAmountOut2");
        emit LogUint(cashAmountOut2);

        require(cashAmountOut == cashAmountOut2, "input not exact to output2");
        require(fees1 == fees2, "fees should be equal 2");
    }

    // Same fees, but diff amount
    function test_inputExactOutputSell1Amount() public {
        inputExactOutputSell1(0,0,0,0,0);
    }

    // Diff amount and Fees
    function test_inputExactOutputSell1FeesAndAmount() public {
        inputExactOutputSell1(0,1106,0,0,0);
    }

    // Diff Fees but same Amount
    function test_inputExactOutputSell2Fees() public {
        inputExactOutputSell2(0,0,1250541351101251073171579362856088277514492256939140234,5749887088669985661716050280910196866583157705802374471,488969518006285946344671806047891173015273936250);
    }
}

Taking example of the last test, test_inputExactOutputSell2Fees, run the test with foundry:

    // Diff Fees but same Amount
    function test_inputExactOutputSell2Fees() public {
        inputExactOutputSell2(0,0,1250541351101251073171579362856088277514492256939140234,5749887088669985661716050280910196866583157705802374471,488969518006285946344671806047891173015273936250);
    }

[FAIL. Reason: revert: fees should be equal 2] test_inputExactOutputSell2() (gas: 38945)
Traces:
  [38945] FeesTest::test_inputExactOutputSell2Fees()
    ├─ emit Log(: "creditPositionId")
    ├─ emit LogUint(: 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77])
    ├─ emit Log(: "tenor")
    ├─ emit LogUint(: 86400 [8.64e4])
    ├─ emit Log(: "amount")
    ├─ emit LogUint(: 69412026960166 [6.941e13])
    ├─ emit Log(: "credit")
    ├─ emit LogUint(: 36431773297927 [3.643e13])
    ├─ emit Log(: "ratePerTenor")
    ├─ emit LogUint(: 67411342690445540 [6.741e16])
    ├─ emit Log(: "Middle creditAmountIn")
    ├─ emit LogUint(: 74092199 [7.409e7])
    ├─ emit Log(: "fees1")
    ├─ emit LogUint(: 950862710 [9.508e8])
    ├─ emit Log(: "fees2")
    ├─ emit LogUint(: 950849685 [9.508e8])
    ├─ emit Log(: "cashAmountOut")
    ├─ emit LogUint(: 69412026960166 [6.941e13])
    ├─ emit Log(: "cashAmountOut2")
    ├─ emit LogUint(: 69412026960166 [6.941e13])
    └─ ← [Revert] revert: fees should be equal 2

Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 932.75µs (85.27µs CPU time)

As observed from Echidna tests, there is a discrepancy in fees: fees1 is calculated as 950,862.710 USDC and fees2 as 950,849.685 USDC when represented with a decimal precision of 1e6.

The difference in fees, diffFees, is computed as follows: diffFees = fees1 − fees2 = 950,862.710 − 950,849.685 = 0.013025

This discrepancy in fees is proportional to the value of params.amount, increasing linearly with its value.

Upon running additional tests, it was noted that:

• In some cases, while the input and output values matched, the fees did not, indicating inconsistencies in fee calculations.
• Conversely, in other scenarios, both assertions for input, output values, and fees failed simultaneously.

The specific test cases implemented to verify these scenarios are:

    // Same fees, but diff amount
    function test_inputExactOutputSell1Amount() public {}

    // Diff amount and Fees
    function test_inputExactOutputSell1FeesAndAmount() public {}

    // Diff Fees but same Amount
    function test_inputExactOutputSell2Fees() public {}

Impact

The discrepancies observed in fees and amounts spanned across different amount ranges, impacting various user types. This inconsistency implies that users may receive or pay different amounts for identical transactions, resulting in unequal treatment among users. Specifically:

• Fee Discrepancy: Users might be charged varying fees for the same transaction based on the path taken through the functions.
• Amount Variance: The amount received or paid by users could differ unexpectedly, potentially causing financial discrepancies.

Recommendations

Update the fee calculation logic to ensure consistency across different scenarios, accurately calculating fees and amounts for all transactions.

If this is not feasible, update the documentation accordingly or list this issue under known issues.

Assessed type

Math

[aviggiano]

Great usage of tooling to show an important invariant of "output amounts should be the same regardless of function parameters used", but the report lacks a proper explanation of the underlying root cause.

Duplicate of #213

[hansfriese]

I agree with the sponsor. Will apply 75% partial credit.

[c4-judge]

hansfriese changed the severity to 3 (High Risk)

[c4-judge]

hansfriese marked the issue as partial-75

[c4-judge]

hansfriese marked the issue as duplicate of #213

[c4-judge]

hansfriese marked the issue as duplicate of #288