`AccountingLibrary.getCreditAmountIn()` calculates swap fees incorrectly.

[c4-bot-3]

Lines of code

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

Vulnerability details

Impact

The protocol would charge less swap fees than expected while selling credit markets.

Proof of Concept

While selling credit markets, getCreditAmountIn() calculates swap fees for a given cash amount out.

File: AccountingLibrary.sol
245:         if (cashAmountOut == maxCashAmountOut) {
246:             // no credit fractionalization
247: 
248:             creditAmountIn = maxCredit;
249:             fees = Math.mulDivUp(cashAmountOut, swapFeePercent, PERCENT); //@audit should divide by PERCENT-swapFeePercent
250:         } else if (cashAmountOut < maxCashAmountOutFragmentation) {
251:             // credit fractionalization
252: 
253:             creditAmountIn = Math.mulDivUp(
254:                 cashAmountOut + state.feeConfig.fragmentationFee, PERCENT + ratePerTenor, PERCENT - swapFeePercent
255:             );
256:             fees = Math.mulDivUp(cashAmountOut, swapFeePercent, PERCENT) + state.feeConfig.fragmentationFee; //@audit should divide by PERCENT-swapFeePercent
257:         } else {

Currently, it calculates by applying swapFeePercent to cashAmountOut but the cashAmountOut means the cash amount that should be paid to the borrower.

When we check executeSellCreditMarket(), total cash paid by a lender is cashAmountOut + fees and fees should be swapFeePercent of this total cash amount. We can confirm this from maxCashAmountOut and maxCredit calculations.

File: SellCreditMarket.sol
169:             cashAmountOut = params.amount;
170: 
171:             (creditAmountIn, fees) = state.getCreditAmountIn({
172:                 cashAmountOut: cashAmountOut,
173:                 maxCashAmountOut: params.creditPositionId == RESERVED_ID
174:                     ? cashAmountOut
175:                     : Math.mulDivDown(creditPosition.credit, PERCENT - state.getSwapFeePercent(tenor), PERCENT + ratePerTenor),
176:                 maxCredit: params.creditPositionId == RESERVED_ID
177:                     ? Math.mulDivUp(cashAmountOut, PERCENT + ratePerTenor, PERCENT - state.getSwapFeePercent(tenor))
178:                     : creditPosition.credit,
179:                 ratePerTenor: ratePerTenor,
180:                 tenor: tenor
181:             });
...
201:         state.data.borrowAToken.transferFrom(params.lender, msg.sender, cashAmountOut);
202:         state.data.borrowAToken.transferFrom(params.lender, state.feeConfig.feeRecipient, fees);

So maxCashAmountOut = creditPosition.credit * (PERCENT / (PERCENT + ratePerTenor) * ((PERCENT - swapFeePercent) / PERCENT) = creditPosition.credit * (PERCENT - swapFeePercent) / (PERCENT + ratePerTenor) - it means fees are swapFeePercent of total cash.

Also, maxCredit = cashAmountOut * ((PERCENT + ratePerTenor) / PERCENT) * (PERCENT / (PERCENT - swapFeePercent)) = cashAmountOut * (PERCENT + ratePerTenor) / (PERCENT - swapFeePercent) - it means the same thing as above.

So fees = swapFeePercent * (cashAmountOut + fees), that is fees = cashAmountOut * swapFeePercent / (1 - swapFeePercent).

But in getCreditAmountIn(), it calculates fees = cashAmountOut * swapFeePercent / PERCENT and the protocol would charge less fees.

Tools Used

Manual Review

Recommended Mitigation Steps

Fees should be calculated like fees = cashAmountOut * swapFeePercent / (PERCENT - swapFeePercent).

 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) {
 // credit fractionalization

 creditAmountIn = Math.mulDivUp(
 cashAmountOut + state.feeConfig.fragmentationFee, PERCENT + ratePerTenor, PERCENT - swapFeePercent
 );
-         fees = Math.mulDivUp(cashAmountOut, swapFeePercent, PERCENT) + state.feeConfig.fragmentationFee;
+        fees = Math.mulDivUp(cashAmountOut, swapFeePercent, PERCENT - swapFeePercent) + state.feeConfig.fragmentationFee;
 }

Assessed type

Math

[c4-judge]

hansfriese marked the issue as unsatisfactory: Invalid

[c4-judge]

hansfriese marked the issue as not a duplicate

[c4-judge]

hansfriese marked the issue as satisfactory

[c4-judge]

hansfriese marked the issue as duplicate of #288