Incorrect calculation of fees lead to protocols Loss - sellCreditMarketOrder.

[howlbot-integration[bot]]

Lines of code

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

Vulnerability details

Impact

Fees are calucated incorrectly and as a result the protocol is receiving less fees than expected.

Proof of Concept

When the operation is exactAmountOut for a sellCreditMarketOrder, that means if the cashAmountOut is the exact cash the seller needs , fees are calculated wrongly.

Here the input parameter , params.amount is the exact cashAmount the seller is receiving after all the fees are deducted.

Now lets check the getCreditAmountIn() used for calculating the creditAmountIn and fees. https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/AccountingLibrary.sol#L228-L263

    function getCreditAmountIn(
       ....
    ) internal view returns (uint256 creditAmountIn, uint256 fees) {
       .....
        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;
        }
        ....
    }

Here in both the cases when the fractionalization is happening and when not happening , fees are calculated by multiplying the cashAmountOut by swapFeePercent and adding the fragmentationFee in the case of fragmentation.

Lets take the case of no fragmentation.

Here cashAmountOut is the amount that the seller is getting after the swapFees are deducted and the protocol is trying to find the swapFees by multiplying the same cashAmountOut again by the swapFeePercent which will return a less and wrong amount.

We can confirm this by checking the way in which maxCashAmountOut is caluclated in executeSellCreditMarket before passing to getCreditAmountIn(). maxCashAmountOut is calculated after deducting the swapfees inorder to compare with the cashAmountOut and the calculation is done inside the function getCreditAmountIn().

c

function executeSellCreditMarket(State storage state, SellCreditMarketParams calldata params)
    external
    returns (uint256 cashAmountOut)
    {
        ....

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

This is the same issue when there is fragmentation.

Tools Used

Manual Review

Recommended Mitigation Steps

if Fragmention -> fees = (creditAmountIn / (1+r) ) swapFeePercent
else -> fees = (creditAmountIn / (1+r) - fragmentationFees) swapFeePercent ( this equation explained in another bug report)

Assessed type

Math

[aviggiano]

Duplicate of #213

[c4-sponsor]

@aviggiano Sponsors can only use these labels: sponsor confirmed, sponsor disputed, sponsor acknowledged.

[c4-judge]

hansfriese marked the issue as satisfactory

[c4-judge]

hansfriese marked the issue as duplicate of #213

[c4-judge]

hansfriese marked the issue as duplicate of #288